From 10b8a65974fe9e59e1fc230cccb37710f0d0d2ee Mon Sep 17 00:00:00 2001 From: Matt Flaherty Date: Thu, 4 Jul 2019 17:44:48 +0100 Subject: [PATCH] Fix for issue #661 --- CHANGELOG.rst | 1 + flask_restplus/namespace.py | 4 ++-- flask_restplus/swagger.py | 3 ++- tests/conftest.py | 14 ++++++++++++- tests/test_swagger.py | 41 +++++++++++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 262b1dda..b675916f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,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) ------------------- diff --git a/flask_restplus/namespace.py b/flask_restplus/namespace.py index 1328ef99..d23a66ff 100644 --- a/flask_restplus/namespace.py +++ b/flask_restplus/namespace.py @@ -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 } @@ -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): ''' diff --git a/flask_restplus/swagger.py b/flask_restplus/swagger.py index 681b08d2..77c364bf 100644 --- a/flask_restplus/swagger.py +++ b/flask_restplus/swagger.py @@ -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 @@ -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 diff --git a/tests/conftest.py b/tests/conftest.py index 753801ce..27b1d914 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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), diff --git a/tests/test_swagger.py b/tests/test_swagger.py index 859994ae..91bccfa5 100644 --- a/tests/test_swagger.py +++ b/tests/test_swagger.py @@ -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,