From fe085c84794bc3398f1616738e2b72fcb9bd8363 Mon Sep 17 00:00:00 2001 From: Ben Steadman Date: Tue, 2 Jul 2019 20:08:45 +0100 Subject: [PATCH] Multiple Routes Swagger Documentation (#658) * multiple route Swagger doc behaviour tests * initial multiple route Swagger doc * 'roll down' doc from @api.route and @api.doc * handle parameter overrides properly * multiple routes with doc unique operationIds * multiple routes Swagger doc per route doc hiding * multiple route alias deprecation test multiple routes params override test * simplify mutiple routes Swagger doc implementation interface * multple routes documentation * python2.7 compatible kwarg --- CHANGELOG.rst | 1 + doc/swagger.rst | 57 ++++++++++++ flask_restplus/api.py | 5 +- flask_restplus/namespace.py | 28 +++--- flask_restplus/swagger.py | 48 +++++++--- tests/test_swagger.py | 175 ++++++++++++++++++++++++++++++++++++ 6 files changed, 290 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 88b1ba4e..262b1dda 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,7 @@ Current - Fix `@api.expect(..., validate=False)` decorators for an :class:`Api` where `validate=True` is set on the constructor (:issue:`609`, :pr:`610`) - 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`` 0.12.1 (2018-09-28) ------------------- diff --git a/doc/swagger.rst b/doc/swagger.rst index 24cb0194..b250692f 100644 --- a/doc/swagger.rst +++ b/doc/swagger.rst @@ -355,6 +355,63 @@ For example, these two declarations are equivalent: def get(self, id): return {} +Multiple Routes per Resource +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Multiple ``Api.route()`` decorators can be used to add multiple routes for a ``Resource``. +The ``doc`` parameter provides documentation **per route**. + +For example, here the ``description`` is applied only to the ``/also-my-resource/`` route: + +.. code-block:: python + + @api.route("/my-resource/") + @api.route( + "/also-my-resource/", + doc={"description": "Alias for /my-resource/"}, + ) + class MyResource(Resource): + def get(self, id): + return {} + +Here, the ``/also-my-resource/`` route is marked as deprecated: + +.. code-block:: python + + @api.route("/my-resource/") + @api.route( + "/also-my-resource/", + doc={ + "description": "Alias for /my-resource/, this route is being phased out in V2", + "deprecated": True, + }, + ) + class MyResource(Resource): + def get(self, id): + return {} + +Documentation applied to the ``Resource`` using ``Api.doc()`` is `shared` amongst all +routes unless explicitly overridden: + +.. code-block:: python + + @api.route("/my-resource/") + @api.route( + "/also-my-resource/", + doc={"description": "Alias for /my-resource/"}, + ) + @api.doc(params={"id": "An ID", description="My resource"}) + class MyResource(Resource): + def get(self, id): + return {} + +Here, the ``id`` documentation from the ``@api.doc()`` decorator is present in both routes, +``/my-resource/`` inherits the ``My resource`` description from the ``@api.doc()`` +decorator and ``/also-my-resource/`` overrides the description with ``Alias for /my-resource/``. + +Routes with a ``doc`` parameter are given a `unique` Swagger ``operationId``. Routes without +``doc`` parameter have the same Swagger ``operationId`` as they are deemed the same operation. + Documenting the fields ---------------------- diff --git a/flask_restplus/api.py b/flask_restplus/api.py index eb44b040..37276503 100644 --- a/flask_restplus/api.py +++ b/flask_restplus/api.py @@ -421,8 +421,9 @@ def add_namespace(self, ns, path=None): if path is not None: self.ns_paths[ns] = path # Register resources - for resource, urls, kwargs in ns.resources: - self.register_resource(ns, resource, *self.ns_urls(ns, urls), **kwargs) + for r in ns.resources: + urls = self.ns_urls(ns, r.urls) + self.register_resource(ns, r.resource, *urls, **r.kwargs) # Register models for name, definition in six.iteritems(ns.models): self.models[name] = definition diff --git a/flask_restplus/namespace.py b/flask_restplus/namespace.py index b3c1c05a..1328ef99 100644 --- a/flask_restplus/namespace.py +++ b/flask_restplus/namespace.py @@ -2,18 +2,22 @@ from __future__ import unicode_literals import inspect -import six import warnings +from collections import namedtuple +import six from flask import request from flask.views import http_method_funcs +from ._http import HTTPStatus from .errors import abort from .marshalling import marshal, marshal_with from .model import Model, OrderedModel, SchemaModel from .reqparse import RequestParser from .utils import merge -from ._http import HTTPStatus + +# Container for each route applied to a Resource using @ns.route decorator +ResourceRoute = namedtuple("ResourceRoute", "resource urls route_doc kwargs") class Namespace(object): @@ -41,7 +45,7 @@ def __init__(self, name, description=None, path=None, decorators=None, validate= self.models = {} self.urls = {} self.decorators = decorators if decorators else [] - self.resources = [] + self.resources = [] # List[ResourceRoute] self.error_handlers = {} self.default_error_handler = None self.authorizations = authorizations @@ -76,7 +80,8 @@ def add_resource(self, resource, *urls, **kwargs): namespace.add_resource(Foo, '/foo', endpoint="foo") namespace.add_resource(FooSpecial, '/special/foo', endpoint="foo") ''' - self.resources.append((resource, urls, kwargs)) + route_doc = kwargs.pop('route_doc', {}) + self.resources.append(ResourceRoute(resource, urls, route_doc, kwargs)) for api in self.apis: ns_urls = api.ns_urls(self, urls) api.register_resource(self, resource, *ns_urls, **kwargs) @@ -88,15 +93,15 @@ def route(self, *urls, **kwargs): def wrapper(cls): doc = kwargs.pop('doc', None) if doc is not None: - self._handle_api_doc(cls, doc) + # build api doc intended only for this route + kwargs['route_doc'] = self._build_doc(cls, doc) self.add_resource(cls, *urls, **kwargs) return cls return wrapper - def _handle_api_doc(self, cls, doc): + def _build_doc(self, cls, doc): if doc is False: - cls.__apidoc__ = False - return + return False unshortcut_params_description(doc) handle_deprecations(doc) for http_method in http_method_funcs: @@ -107,7 +112,7 @@ def _handle_api_doc(self, cls, doc): handle_deprecations(doc[http_method]) if 'expect' in doc[http_method] and not isinstance(doc[http_method]['expect'], (list, tuple)): doc[http_method]['expect'] = [doc[http_method]['expect']] - cls.__apidoc__ = merge(getattr(cls, '__apidoc__', {}), doc) + return merge(getattr(cls, '__apidoc__', {}), doc) def doc(self, shortcut=None, **kwargs): '''A decorator to add some api documentation to the decorated object''' @@ -116,7 +121,10 @@ def doc(self, shortcut=None, **kwargs): show = shortcut if isinstance(shortcut, bool) else True def wrapper(documented): - self._handle_api_doc(documented, kwargs if show else False) + documented.__apidoc__ = self._build_doc( + documented, + kwargs if show else False + ) return documented return wrapper diff --git a/flask_restplus/swagger.py b/flask_restplus/swagger.py index d98cb5d0..681b08d2 100644 --- a/flask_restplus/swagger.py +++ b/flask_restplus/swagger.py @@ -132,12 +132,15 @@ def parse_docstring(obj): return parsed -def is_hidden(resource): +def is_hidden(resource, route_doc=None): ''' Determine whether a Resource has been hidden from Swagger documentation i.e. by using Api.doc(False) decorator ''' - return hasattr(resource, "__apidoc__") and resource.__apidoc__ is False + if route_doc is False: + return True + else: + return hasattr(resource, "__apidoc__") and resource.__apidoc__ is False class Swagger(object): @@ -184,9 +187,17 @@ def as_dict(self): responses = self.register_errors() for ns in self.api.namespaces: - for resource, urls, kwargs in ns.resources: + for resource, urls, route_doc, kwargs in ns.resources: for url in self.api.ns_urls(ns, urls): - paths[extract_path(url)] = self.serialize_resource(ns, resource, url, kwargs) + path = extract_path(url) + serialized = self.serialize_resource( + ns, + resource, + url, + route_doc=route_doc, + **kwargs + ) + paths[path] = serialized # merge in the top-level authorizations for ns in self.api.namespaces: @@ -236,8 +247,10 @@ def extract_tags(self, api): if not ns.resources: continue # hide namespaces with all Resources hidden from Swagger documentation - resources = (resource for resource, urls, kwargs in ns.resources) - if all(is_hidden(r) for r in resources): + if all( + is_hidden(r.resource, route_doc=r.route_doc) + for r in ns.resources + ): continue if ns.name not in by_name: tags.append({ @@ -248,11 +261,22 @@ def extract_tags(self, api): by_name[ns.name]['description'] = ns.description return tags - def extract_resource_doc(self, resource, url): - doc = getattr(resource, '__apidoc__', {}) + def extract_resource_doc(self, resource, url, route_doc=None): + route_doc = {} if route_doc is None else route_doc + if route_doc is False: + return False + doc = merge(getattr(resource, '__apidoc__', {}), route_doc) if doc is False: return False - doc['name'] = resource.__name__ + + # ensure unique names for multiple routes to the same resource + # provides different Swagger operationId's + doc["name"] = ( + "{}_{}".format(resource.__name__, url) + if route_doc + else resource.__name__ + ) + params = merge(self.expected_params(doc), doc.get('params', OrderedDict())) params = merge(params, extract_path_params(url)) # Track parameters for late deduplication @@ -349,8 +373,8 @@ def register_errors(self): responses[exception.__name__] = not_none(response) return responses - def serialize_resource(self, ns, resource, url, kwargs): - doc = self.extract_resource_doc(resource, url) + def serialize_resource(self, ns, resource, url, route_doc=None, **kwargs): + doc = self.extract_resource_doc(resource, url, route_doc=route_doc) if doc is False: return path = { @@ -405,7 +429,7 @@ def description_for(self, doc, method): '''Extract the description metadata and fallback on the whole docstring''' parts = [] if 'description' in doc: - parts.append(doc['description']) + parts.append(doc['description'] or "") if method in doc and 'description' in doc[method]: parts.append(doc[method]['description']) if doc[method]['docstring']['details']: diff --git a/tests/test_swagger.py b/tests/test_swagger.py index cd28fd6f..859994ae 100644 --- a/tests/test_swagger.py +++ b/tests/test_swagger.py @@ -3049,6 +3049,181 @@ def post(self): assert 'get' in path assert 'post' not in path + def test_multiple_routes_inherit_doc(self, api, client): + @api.route('/foo/bar') + @api.route('/bar') + @api.doc(description='an endpoint') + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['description'] == 'an endpoint' + + path = data['paths']['/bar'] + assert path['get']['description'] == 'an endpoint' + + def test_multiple_routes_individual_doc(self, api, client): + @api.route('/foo/bar', doc={'description': 'the same endpoint'}) + @api.route('/bar', doc={'description': 'an endpoint'}) + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['description'] == 'the same endpoint' + + path = data['paths']['/bar'] + assert path['get']['description'] == 'an endpoint' + + def test_multiple_routes_override_doc(self, api, client): + @api.route('/foo/bar', doc={'description': 'the same endpoint'}) + @api.route('/bar') + @api.doc(description='an endpoint') + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['description'] == 'the same endpoint' + + path = data['paths']['/bar'] + assert path['get']['description'] == 'an endpoint' + + def test_multiple_routes_no_doc_same_operationIds(self, api, client): + @api.route('/foo/bar') + @api.route('/bar') + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + expected_operation_id = 'get_test_resource' + + path = data['paths']['/foo/bar'] + assert path['get']['operationId'] == expected_operation_id + + path = data['paths']['/bar'] + assert path['get']['operationId'] == expected_operation_id + + def test_multiple_routes_with_doc_unique_operationIds(self, api, client): + @api.route( + "/foo/bar", + doc={"description": "I should be treated separately"}, + ) + @api.route("/bar") + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['operationId'] == 'get_test_resource_/foo/bar' + + path = data['paths']['/bar'] + assert path['get']['operationId'] == 'get_test_resource' + + def test_mutltiple_routes_merge_doc(self, api, client): + @api.route('/foo/bar', doc={'description': 'the same endpoint'}) + @api.route('/bar', doc={'description': False}) + @api.doc(security=[{'oauth2': ['read', 'write']}]) + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['description'] == 'the same endpoint' + assert path['get']['security'] == [{'oauth2': ['read', 'write']}] + + path = data['paths']['/bar'] + assert 'description' not in path['get'] + assert path['get']['security'] == [{'oauth2': ['read', 'write']}] + + def test_multiple_routes_deprecation(self, api, client): + @api.route('/foo/bar', doc={'deprecated': True}) + @api.route('/bar') + class TestResource(restplus.Resource): + def get(self): + pass + + data = client.get_specs() + + path = data['paths']['/foo/bar'] + assert path['get']['deprecated'] is True + + path = data['paths']['/bar'] + assert 'deprecated' not in path['get'] + + @pytest.mark.parametrize('path_name', ['/name/{age}/', '/first-name/{age}/']) + def test_multiple_routes_explicit_parameters_override(self, path_name, api, client): + @api.route("/name//", endpoint="by-name") + @api.route("/first-name//") + @api.doc( + params={ + "q": { + "type": "string", + "in": "query", + "description": "Overriden description", + }, + "age": {"description": "An age"}, + } + ) + class ByNameResource(restplus.Resource): + @api.doc( + params={"q": {"description": "A query string"}} + ) + def get(self, age): + return {} + + def post(self, age): + pass + + data = client.get_specs() + assert path_name in data['paths'] + + path = data['paths'][path_name] + assert len(path['parameters']) == 1 + + by_name = dict((p['name'], p) for p in path['parameters']) + + parameter = by_name['age'] + assert parameter['name'] == 'age' + assert parameter['type'] == 'integer' + assert parameter['in'] == 'path' + assert parameter['required'] is True + assert parameter['description'] == 'An age' + + # Don't duplicate parameters + assert 'q' not in by_name + + get = path['get'] + assert len(get['parameters']) == 1 + + parameter = get['parameters'][0] + assert parameter['name'] == 'q' + assert parameter['type'] == 'string' + assert parameter['in'] == 'query' + assert parameter['description'] == 'A query string' + + post = path['post'] + assert len(post['parameters']) == 1 + + parameter = post['parameters'][0] + assert parameter['name'] == 'q' + assert parameter['type'] == 'string' + assert parameter['in'] == 'query' + assert parameter['description'] == 'Overriden description' + class SwaggerDeprecatedTest(object): def test_doc_parser_parameters(self, api):