Skip to content

Commit

Permalink
Merge pull request noirbizarre#663 from flayman/661-api-response-code…
Browse files Browse the repository at this point in the history
…-as-string

Fix for issue noirbizarre#661
  • Loading branch information
j5awry authored Sep 11, 2019
2 parents fe9fec6 + 10b8a65 commit ddc2aee
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Current
- Ensure `basePath` is always a path
- Hide Namespaces with all hidden Resources from Swagger documentation
- Per route Swagger documentation for multiple routes on a ``Resource``
- Fix Swagger `duplicate mapping key` problem from conflicts between response codes given as string or integer (:issue`661`)

0.12.1 (2018-09-28)
-------------------
Expand Down
4 changes: 2 additions & 2 deletions flask_restplus/namespace.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def marshal_with(self, fields, as_list=False, code=HTTPStatus.OK, description=No
def wrapper(func):
doc = {
'responses': {
code: (description, [fields]) if as_list else (description, fields)
str(code): (description, [fields]) if as_list else (description, fields)
},
'__mask__': kwargs.get('mask', True), # Mask values can't be determined outside app context
}
Expand Down Expand Up @@ -289,7 +289,7 @@ def response(self, code, description, model=None, **kwargs):
:param ModelBase model: an optional response model
'''
return self.doc(responses={code: (description, model, kwargs)})
return self.doc(responses={str(code): (description, model, kwargs)})

def header(self, name, description=None, **kwargs):
'''
Expand Down
3 changes: 2 additions & 1 deletion flask_restplus/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ def responses_for(self, doc, method):
for d in doc, doc[method]:
if 'responses' in d:
for code, response in iteritems(d['responses']):
code = str(code)
if isinstance(response, string_types):
description = response
model = None
Expand Down Expand Up @@ -514,7 +515,7 @@ def responses_for(self, doc, method):
for name, description in iteritems(d['docstring']['raises']):
for exception, handler in iteritems(self.api.error_handlers):
error_responses = getattr(handler, '__apidoc__', {}).get('responses', {})
code = list(error_responses.keys())[0] if error_responses else None
code = str(list(error_responses.keys())[0]) if error_responses else None
if code and exception.__name__ == name:
responses[code] = {'$ref': '#/responses/{0}'.format(name)}
break
Expand Down
14 changes: 13 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,23 @@


class TestClient(FlaskClient):
# Borrowed from https://pythonadventures.wordpress.com/2016/03/06/detect-duplicate-keys-in-a-json-file/
# Thank you to Wordpress author @ubuntuincident, aka Jabba Laci.
def dict_raise_on_duplicates(self, ordered_pairs):
"""Reject duplicate keys."""
d = {}
for k, v in ordered_pairs:
if k in d:
raise ValueError("duplicate key: %r" % (k,))
else:
d[k] = v
return d

def get_json(self, url, status=200, **kwargs):
response = self.get(url, **kwargs)
assert response.status_code == status
assert response.content_type == 'application/json'
return json.loads(response.data.decode('utf8'))
return json.loads(response.data.decode('utf8'), object_pairs_hook=self.dict_raise_on_duplicates)

def post_json(self, url, data, status=200, **kwargs):
response = self.post(url, data=json.dumps(data),
Expand Down
41 changes: 41 additions & 0 deletions tests/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,47 @@ def get(self):

client.get_specs(status=500)

def test_specs_no_duplicate_response_keys(self, api, client):
'''
This tests that the swagger.json document will not be written with duplicate object keys
due to the coercion of dict keys to string. The last @api.response should win.
'''
# Note the use of a strings '404' and '200' in class decorators as opposed to ints in method decorators.
@api.response('404', 'Not Found')
class BaseResource(restplus.Resource):
def get(self):
pass

model = api.model('SomeModel', {
'message': restplus.fields.String,
})

@api.route('/test/')
@api.response('200', 'Success')
class TestResource(BaseResource):
# @api.marshal_with also yields a response
@api.marshal_with(model, code=200, description='Success on method')
@api.response(404, 'Not Found on method')
def get(self):
{}

data = client.get_specs('')
paths = data['paths']

op = paths['/test/']['get']
print(op['responses'])
assert op['responses'] == {
'200': {
'description': 'Success on method',
'schema': {
'$ref': '#/definitions/SomeModel'
}
},
'404': {
'description': 'Not Found on method',
}
}

def test_clone(self, api, client):
parent = api.model('Person', {
'name': restplus.fields.String,
Expand Down

0 comments on commit ddc2aee

Please sign in to comment.