Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle responses as JSON only if required to. #175

Merged
merged 1 commit into from
Jun 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Changelog
=========

- The content will be used to build the Changelog for the new bravado-core release
- Fix handling of API calls that return non-JSON content (specifically text content).
(add before this line your modifications summary and PR reference)

4.7.3 (2017-05-05)
Expand Down
18 changes: 11 additions & 7 deletions bravado_core/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,19 @@ def has_content(response_spec):
if not has_content(response_spec):
return None

# TODO: Non-json response contents
content_spec = deref(response_spec['schema'])
content_value = response.json()
content_type = response.headers.get('content-type', '').lower()

if content_type.startswith(APP_JSON):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you went with startswith instead of equality?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To opportunistically capture "application/json-rpc" should that ever occur? Admittedly this test may need to evolve over time since there are various other content types that could contain json. I don't feel strongly about this either way.

content_spec = deref(response_spec['schema'])
content_value = response.json()

if op.swagger_spec.config['validate_responses']:
validate_schema_object(op.swagger_spec, content_spec, content_value)
if op.swagger_spec.config['validate_responses']:
validate_schema_object(op.swagger_spec, content_spec, content_value)

return unmarshal_schema_object(
op.swagger_spec, content_spec, content_value)
return unmarshal_schema_object(
op.swagger_spec, content_spec, content_value)
# TODO: Non-json response contents
return response.text


def get_response_spec(status_code, op):
Expand Down
17 changes: 17 additions & 0 deletions tests/response/unmarshal_response_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from mock import Mock
from mock import patch

from bravado_core.content_type import APP_JSON
from bravado_core.response import IncomingResponse
from bravado_core.response import unmarshal_response

Expand Down Expand Up @@ -34,6 +35,7 @@ def test_json_content(empty_swagger_spec, response_spec):
response = Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_JSON},
json=Mock(return_value='Monday'))

with patch('bravado_core.response.get_response_spec') as m:
Expand All @@ -42,11 +44,25 @@ def test_json_content(empty_swagger_spec, response_spec):
assert 'Monday' == unmarshal_response(response, op)


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

with patch('bravado_core.response.get_response_spec') as m:
m.return_value = response_spec
op = Mock(swagger_spec=empty_swagger_spec)
assert 'Monday' == unmarshal_response(response, op)


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

with patch('bravado_core.response.validate_schema_object') as val_schem:
Expand All @@ -62,6 +78,7 @@ def test_performs_validation(empty_swagger_spec, response_spec):
response = Mock(
spec=IncomingResponse,
status_code=200,
headers={'content-type': APP_JSON},
json=Mock(return_value='Monday'))

with patch('bravado_core.response.validate_schema_object') as val_schem:
Expand Down