Skip to content

Commit

Permalink
Merge pull request #336 from macisamuele/maci-response-handling-is-pe…
Browse files Browse the repository at this point in the history
…rformed-in-bravado

Response handling is performed in bravado
  • Loading branch information
macisamuele authored Dec 6, 2017
2 parents 7128147 + 3b4c060 commit c524bce
Show file tree
Hide file tree
Showing 6 changed files with 209 additions and 45 deletions.
24 changes: 3 additions & 21 deletions bravado/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
from six import iteritems
from six import itervalues

from bravado.config_defaults import CONFIG_DEFAULTS
from bravado.config_defaults import REQUEST_OPTIONS_DEFAULTS
from bravado.docstring_property import docstring_property
from bravado.requests_client import RequestsClient
from bravado.swagger_model import Loader
Expand All @@ -61,25 +63,6 @@
log = logging.getLogger(__name__)


CONFIG_DEFAULTS = {
# See the constructor of :class:`bravado.http_future.HttpFuture` for an
# in depth explanation of what this means.
'also_return_response': False,
}

REQUEST_OPTIONS_DEFAULTS = {
# List of callbacks that are executed after the incoming response has been
# validated and the swagger_result has been unmarshalled.
#
# The callback should expect two arguments:
# param : incoming_response
# type : subclass of class:`bravado_core.response.IncomingResponse`
# param : operation
# type : class:`bravado_core.operation.Operation`
'response_callbacks': [],
}


class SwaggerClient(object):
"""A client for accessing a Swagger-documented RESTful service.
Expand All @@ -91,8 +74,7 @@ def __init__(self, swagger_spec, also_return_response=False):
self.swagger_spec = swagger_spec

@classmethod
def from_url(cls, spec_url, http_client=None, request_headers=None,
config=None):
def from_url(cls, spec_url, http_client=None, request_headers=None, config=None):
"""Build a :class:`SwaggerClient` from a url to the Swagger
specification for a RESTful API.
Expand Down
18 changes: 18 additions & 0 deletions bravado/config_defaults.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# -*- coding: utf-8 -*-
CONFIG_DEFAULTS = {
# See the constructor of :class:`bravado.http_future.HttpFuture` for an
# in depth explanation of what this means.
'also_return_response': False,
}

REQUEST_OPTIONS_DEFAULTS = {
# List of callbacks that are executed after the incoming response has been
# validated and the swagger_result has been unmarshalled.
#
# The callback should expect two arguments:
# param : incoming_response
# type : subclass of class:`bravado_core.response.IncomingResponse`
# param : operation
# type : class:`bravado_core.operation.Operation`
'response_callbacks': [],
}
57 changes: 49 additions & 8 deletions bravado/http_future.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@
import sys
from functools import wraps

import bravado_core
import six
import umsgpack
from bravado_core.content_type import APP_JSON
from bravado_core.content_type import APP_MSGPACK
from bravado_core.exception import MatchingResponseNotFound
from bravado_core.response import get_response_spec
from bravado_core.unmarshal import unmarshal_schema_object
from bravado_core.validate import validate_schema_object

from bravado.config_defaults import REQUEST_OPTIONS_DEFAULTS
from bravado.exception import BravadoTimeoutError
from bravado.exception import make_http_exception

Expand Down Expand Up @@ -92,7 +98,7 @@ def __init__(self, future, response_adapter, operation=None,
self.future = future
self.response_adapter = response_adapter
self.operation = operation
self.response_callbacks = response_callbacks or []
self.response_callbacks = response_callbacks or REQUEST_OPTIONS_DEFAULTS['response_callbacks']
self.also_return_response = also_return_response

@reraise_errors
Expand Down Expand Up @@ -130,12 +136,10 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None):
This hands the response over to bravado_core for validation and
unmarshalling and then runs any response callbacks. On success, the
swagger_result is available as ``incoming_response.swagger_result``.
:type incoming_response: :class:`bravado_core.response.IncomingResponse`
:type operation: :class:`bravado_core.operation.Operation`
:type response_callbacks: list of callable. See
bravado_core.client.REQUEST_OPTIONS_DEFAULTS.
:raises: HTTPError
- On 5XX status code, the HTTPError has minimal information.
- On non-2XX status code with no matching response, the HTTPError
Expand All @@ -147,10 +151,10 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None):

try:
raise_on_unexpected(incoming_response)
incoming_response.swagger_result = \
bravado_core.response.unmarshal_response(
incoming_response,
operation)
incoming_response.swagger_result = unmarshal_response_inner(
response=incoming_response,
op=operation,
)
except MatchingResponseNotFound as e:
exception = make_http_exception(
response=incoming_response,
Expand All @@ -168,6 +172,43 @@ def unmarshal_response(incoming_response, operation, response_callbacks=None):
raise_on_expected(incoming_response)


def unmarshal_response_inner(response, op):
"""
Unmarshal incoming http response into a value based on the
response specification.
:type response: :class:`bravado_core.response.IncomingResponse`
:type op: :class:`bravado_core.operation.Operation`
:returns: value where type(value) matches response_spec['schema']['type']
if it exists, None otherwise.
"""
deref = op.swagger_spec.deref
response_spec = get_response_spec(status_code=response.status_code, op=op)

if 'schema' not in response_spec:
return None

content_type = response.headers.get('content-type', '').lower()

if content_type.startswith(APP_JSON) or content_type.startswith(APP_MSGPACK):
content_spec = deref(response_spec['schema'])
if content_type.startswith(APP_JSON):
content_value = response.json()
else:
content_value = umsgpack.unpackb(response.raw_bytes)

if op.swagger_spec.config.get('validate_responses', False):
validate_schema_object(op.swagger_spec, content_spec, content_value)

return unmarshal_schema_object(
swagger_spec=op.swagger_spec,
schema_object_spec=content_spec,
value=content_value,
)

# TODO: Non-json response contents
return response.text


def raise_on_unexpected(http_response):
"""Raise an HTTPError if the response is 5XX.
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"Programming Language :: Python :: 3.6",
],
install_requires=[
"bravado-core >= 4.2.2",
"bravado-core >= 4.11.0",
"msgpack-python",
"python-dateutil",
"pyyaml",
"requests >= 2",
Expand Down
119 changes: 119 additions & 0 deletions tests/http_future/unmarshall_response_inner_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# -*- coding: utf-8 -*-
import mock
import msgpack
import pytest
from bravado_core.content_type import APP_JSON
from bravado_core.content_type import APP_MSGPACK
from bravado_core.response import IncomingResponse
from bravado_core.spec import Spec

from bravado.http_future import unmarshal_response_inner


@pytest.fixture
def empty_swagger_spec():
return Spec(spec_dict={})


@pytest.fixture
def response_spec():
return {
'description': "Day of the week",
'schema': {
'type': 'string',
}
}


@pytest.fixture
def mock_get_response_spec():
with mock.patch('bravado.http_future.get_response_spec') as m:
yield m


@pytest.fixture
def mock_validate_schema_object():
with mock.patch('bravado.http_future.validate_schema_object') as m:
yield m


def test_no_content(mock_get_response_spec, empty_swagger_spec):
response_spec = {
'description': "I don't have a 'schema' key so I return nothing",
}
response = mock.Mock(spec=IncomingResponse, status_code=200)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
result = unmarshal_response_inner(response, op)
assert result is None


def test_json_content(mock_get_response_spec, empty_swagger_spec, response_spec):
response = mock.Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_JSON},
json=mock.Mock(return_value='Monday'),
)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
assert 'Monday' == unmarshal_response_inner(response, op)


def test_msgpack_content(mock_get_response_spec, empty_swagger_spec, response_spec):
message = 'Monday'
response = mock.Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_MSGPACK},
raw_bytes=msgpack.dumps(message),
)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
assert message == unmarshal_response_inner(response, op)


def test_text_content(mock_get_response_spec, empty_swagger_spec, response_spec):
response = mock.Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': 'text/plain'},
text='Monday',
)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
assert 'Monday' == unmarshal_response_inner(response, op)


def test_skips_validation(mock_validate_schema_object, mock_get_response_spec, empty_swagger_spec, response_spec):
empty_swagger_spec.config['validate_responses'] = False
response = mock.Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_JSON},
json=mock.Mock(return_value='Monday'),
)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
unmarshal_response_inner(response, op)
assert mock_validate_schema_object.call_count == 0


def test_performs_validation(mock_validate_schema_object, mock_get_response_spec, empty_swagger_spec, response_spec):
empty_swagger_spec.config['validate_responses'] = True
response = mock.Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_JSON},
json=mock.Mock(return_value='Monday'),
)

mock_get_response_spec.return_value = response_spec
op = mock.Mock(swagger_spec=empty_swagger_spec)
unmarshal_response_inner(response, op)
assert mock_validate_schema_object.call_count == 1
33 changes: 18 additions & 15 deletions tests/http_future/unmarshall_response_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
from bravado.http_future import unmarshal_response


@pytest.fixture
def mock_unmarshal_response_inner():
with patch('bravado.http_future.unmarshal_response_inner') as m:
yield m


def test_5XX():
incoming_response = Mock(spec=IncomingResponse, status_code=500)
operation = Mock(spec=Operation)
Expand All @@ -18,18 +24,17 @@ def test_5XX():
assert excinfo.value.response.status_code == 500


@patch('bravado_core.response.unmarshal_response', return_value=99)
def test_2XX(_1):
def test_2XX(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.return_value = 99
incoming_response = Mock(spec=IncomingResponse)
incoming_response.status_code = 200
operation = Mock(spec=Operation)
unmarshal_response(incoming_response, operation)
assert incoming_response.swagger_result == 99


@patch('bravado_core.response.unmarshal_response',
side_effect=MatchingResponseNotFound('boo'))
def test_2XX_matching_response_not_found_in_spec(_1):
def test_2XX_matching_response_not_found_in_spec(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.side_effect = MatchingResponseNotFound('boo')
incoming_response = Mock(spec=IncomingResponse, status_code=200)
operation = Mock(spec=Operation)
with pytest.raises(HTTPError) as excinfo:
Expand All @@ -38,19 +43,17 @@ def test_2XX_matching_response_not_found_in_spec(_1):
assert excinfo.value.message == 'boo'


@patch('bravado_core.response.unmarshal_response',
side_effect=MatchingResponseNotFound)
def test_4XX_matching_response_not_found_in_spec(_1):
def test_4XX_matching_response_not_found_in_spec(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.side_effect = MatchingResponseNotFound
incoming_response = Mock(spec=IncomingResponse, status_code=404)
operation = Mock(spec=Operation)
with pytest.raises(HTTPError) as excinfo:
unmarshal_response(incoming_response, operation)
assert excinfo.value.response.status_code == 404


@patch('bravado_core.response.unmarshal_response',
return_value={'msg': 'Not found'})
def test_4XX(_1):
def test_4XX(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.return_value = {'msg': 'Not found'}
incoming_response = Mock(spec=IncomingResponse, status_code=404)
operation = Mock(spec=Operation)
with pytest.raises(HTTPError) as excinfo:
Expand All @@ -59,8 +62,8 @@ def test_4XX(_1):
assert excinfo.value.swagger_result == {'msg': 'Not found'}


@patch('bravado_core.response.unmarshal_response', return_value=99)
def test_response_callbacks_executed_on_happy_path(_1):
def test_response_callbacks_executed_on_happy_path(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.return_value = 99
incoming_response = Mock(spec=IncomingResponse)
incoming_response.status_code = 200
operation = Mock(spec=Operation)
Expand All @@ -71,8 +74,8 @@ def test_response_callbacks_executed_on_happy_path(_1):
assert callback.call_count == 1


@patch('bravado_core.response.unmarshal_response', return_value=99)
def test_response_callbacks_executed_on_failure(_1):
def test_response_callbacks_executed_on_failure(mock_unmarshal_response_inner):
mock_unmarshal_response_inner.return_value = 99
incoming_response = Mock(spec=IncomingResponse, status_code=404)
operation = Mock(spec=Operation)
callback = Mock()
Expand Down

0 comments on commit c524bce

Please sign in to comment.