From 7b1582e00e91001ec07cd394520ec5cdd2c2add1 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 20 Sep 2017 11:29:47 +0200 Subject: [PATCH] Allow `schema = None`. Deprecate `exclude_from_schema` (#5422) * Add tests for schema exclusions * Move exclusion check to should_include_endpoint * Update docs * Switch to using `schema = None` * Test PendingDeprecationWarnings * Add note to release notes. * s/deprecated/pending deprecation/ * Add PR link to release notes * Correct typo in test class name * Test 'exclude_from_schema' deprecation warning message (#1) * Correct deprecation warning message --- docs/api-guide/schemas.md | 6 ++ docs/api-guide/views.md | 17 +++-- docs/topics/release-notes.md | 3 + rest_framework/decorators.py | 10 ++- rest_framework/routers.py | 2 +- rest_framework/schemas/generators.py | 14 +++- rest_framework/schemas/views.py | 2 +- rest_framework/views.py | 2 - tests/test_schemas.py | 109 ++++++++++++++++++++++++++- 9 files changed, 149 insertions(+), 16 deletions(-) diff --git a/docs/api-guide/schemas.md b/docs/api-guide/schemas.md index f913f046f6..fc848199ca 100644 --- a/docs/api-guide/schemas.md +++ b/docs/api-guide/schemas.md @@ -225,6 +225,12 @@ To customise the `Link` generation you may: This allows manually specifying the schema for some views whilst maintaining automatic generation elsewhere. +You may disable schema generation for a view by setting `schema` to `None`: + + class CustomView(APIView): + ... + schema = None # Will not appear in schema + --- **Note**: For full details on `SchemaGenerator` plus the `AutoSchema` and diff --git a/docs/api-guide/views.md b/docs/api-guide/views.md index 24dd42578e..2f9f51cf92 100644 --- a/docs/api-guide/views.md +++ b/docs/api-guide/views.md @@ -130,7 +130,7 @@ REST framework also allows you to work with regular function based views. It pr ## @api_view() -**Signature:** `@api_view(http_method_names=['GET'], exclude_from_schema=False)` +**Signature:** `@api_view(http_method_names=['GET'])` The core of this functionality is the `api_view` decorator, which takes a list of HTTP methods that your view should respond to. For example, this is how you would write a very simple view that just manually returns some data: @@ -150,12 +150,6 @@ By default only `GET` methods will be accepted. Other methods will respond with return Response({"message": "Got some data!", "data": request.data}) return Response({"message": "Hello, world!"}) -You can also mark an API view as being omitted from any [auto-generated schema][schemas], -using the `exclude_from_schema` argument.: - - @api_view(['GET'], exclude_from_schema=True) - def api_docs(request): - ... ## API policy decorators @@ -204,7 +198,14 @@ decorator. For example: return Response({"message": "Hello for today! See you tomorrow!"}) This decorator takes a single `AutoSchema` instance, an `AutoSchema` subclass -instance or `ManualSchema` instance as described in the [Schemas documentation][schemas], +instance or `ManualSchema` instance as described in the [Schemas documentation][schemas]. +You may pass `None` in order to exclude the view from schema generation. + + @api_view(['GET']) + @schema(None) + def view(request): + return Response({"message": "Will not appear in schema!"}) + [cite]: http://reinout.vanrees.org/weblog/2011/08/24/class-based-views-usage.html [cite2]: http://www.boredomandlaziness.org/2012/05/djangos-cbvs-are-not-mistake-but.html diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index f4617ac230..b9fff2ce0c 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -43,6 +43,8 @@ You can determine your currently installed version using `pip freeze`: ### 3.6.5 * Fix `DjangoModelPermissions` to ensure user authentication before calling the view's `get_queryset()` method. As a side effect, this changes the order of the HTTP method permissions and authentication checks, and 405 responses will only be returned when authenticated. If you want to replicate the old behavior, see the PR for details. [#5376][gh5376] +* Deprecated `exclude_from_schema` on `APIView` and `api_view` decorator. Set `schema = None` or `@schema(None)` as appropriate. [#5422][gh5422] + ### 3.6.4 @@ -1423,3 +1425,4 @@ For older release notes, [please see the version 2.x documentation][old-release- [gh5376]: https://github.com/encode/django-rest-framework/issues/5376 +[gh5422]: https://github.com/encode/django-rest-framework/issues/5422 diff --git a/rest_framework/decorators.py b/rest_framework/decorators.py index 1297f96b4c..cdbd59e998 100644 --- a/rest_framework/decorators.py +++ b/rest_framework/decorators.py @@ -9,6 +9,7 @@ from __future__ import unicode_literals import types +import warnings from django.utils import six @@ -75,7 +76,14 @@ def handler(self, *args, **kwargs): WrappedAPIView.schema = getattr(func, 'schema', APIView.schema) - WrappedAPIView.exclude_from_schema = exclude_from_schema + if exclude_from_schema: + warnings.warn( + "The `exclude_from_schema` argument to `api_view` is pending deprecation. " + "Use the `schema` decorator instead, passing `None`.", + PendingDeprecationWarning + ) + WrappedAPIView.exclude_from_schema = exclude_from_schema + return WrappedAPIView.as_view() return decorator diff --git a/rest_framework/routers.py b/rest_framework/routers.py index 01daa7e7d4..3b5ef46d83 100644 --- a/rest_framework/routers.py +++ b/rest_framework/routers.py @@ -291,7 +291,7 @@ class APIRootView(views.APIView): The default basic root view for DefaultRouter """ _ignore_model_permissions = True - exclude_from_schema = True + schema = None # exclude from schema api_root_dict = None def get(self, request, *args, **kwargs): diff --git a/rest_framework/schemas/generators.py b/rest_framework/schemas/generators.py index 8344f64f0e..3e927527cb 100644 --- a/rest_framework/schemas/generators.py +++ b/rest_framework/schemas/generators.py @@ -3,6 +3,7 @@ See schemas.__init__.py for package overview. """ +import warnings from collections import OrderedDict from importlib import import_module @@ -148,6 +149,17 @@ def should_include_endpoint(self, path, callback): if not is_api_view(callback): return False # Ignore anything except REST framework views. + if hasattr(callback.cls, 'exclude_from_schema'): + fmt = ("The `{}.exclude_from_schema` attribute is pending deprecation. " + "Set `schema = None` instead.") + msg = fmt.format(callback.cls.__name__) + warnings.warn(msg, PendingDeprecationWarning) + if getattr(callback.cls, 'exclude_from_schema', False): + return False + + if callback.cls.schema is None: + return False + if path.endswith('.{format}') or path.endswith('.{format}/'): return False # Ignore .json style URLs. @@ -239,8 +251,6 @@ def get_links(self, request=None): view_endpoints = [] for path, method, callback in self.endpoints: view = self.create_view(callback, method, request) - if getattr(view, 'exclude_from_schema', False): - continue path = self.coerce_path(path, method, view) paths.append(path) view_endpoints.append((path, method, view)) diff --git a/rest_framework/schemas/views.py b/rest_framework/schemas/views.py index 932b5a4871..b13eadea9c 100644 --- a/rest_framework/schemas/views.py +++ b/rest_framework/schemas/views.py @@ -11,7 +11,7 @@ class SchemaView(APIView): _ignore_model_permissions = True - exclude_from_schema = True + schema = None # exclude from schema renderer_classes = None schema_generator = None public = False diff --git a/rest_framework/views.py b/rest_framework/views.py index ccc2047eec..dfed158887 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -112,8 +112,6 @@ class APIView(View): # Allow dependency injection of other settings to make testing easier. settings = api_settings - # Mark the view as being included or excluded from schema generation. - exclude_from_schema = False schema = AutoSchema() @classmethod diff --git a/tests/test_schemas.py b/tests/test_schemas.py index f8a63aa897..184401a868 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -8,12 +8,15 @@ from rest_framework import filters, pagination, permissions, serializers from rest_framework.compat import coreapi, coreschema -from rest_framework.decorators import detail_route, list_route +from rest_framework.decorators import ( + api_view, detail_route, list_route, schema +) from rest_framework.request import Request from rest_framework.routers import DefaultRouter from rest_framework.schemas import ( AutoSchema, ManualSchema, SchemaGenerator, get_schema_view ) +from rest_framework.schemas.generators import EndpointEnumerator from rest_framework.test import APIClient, APIRequestFactory from rest_framework.utils import formatting from rest_framework.views import APIView @@ -613,3 +616,107 @@ def post(self, request, *args, **kwargs): descr = schema.get_description('example', 'get') # the first and last character are '\n' correctly removed by get_description assert descr == formatting.dedent(ExampleDocstringAPIView.__doc__[1:][:-1]) + + +# Views for SchemaGenerationExclusionTests +class ExcludedAPIView(APIView): + schema = None + + def get(self, request, *args, **kwargs): + pass + + +@api_view(['GET']) +@schema(None) +def excluded_fbv(request): + pass + + +@api_view(['GET']) +def included_fbv(request): + pass + + +@unittest.skipUnless(coreapi, 'coreapi is not installed') +class SchemaGenerationExclusionTests(TestCase): + def setUp(self): + self.patterns = [ + url('^excluded-cbv/$', ExcludedAPIView.as_view()), + url('^excluded-fbv/$', excluded_fbv), + url('^included-fbv/$', included_fbv), + ] + + def test_schema_generator_excludes_correctly(self): + """Schema should not include excluded views""" + generator = SchemaGenerator(title='Exclusions', patterns=self.patterns) + schema = generator.get_schema() + expected = coreapi.Document( + url='', + title='Exclusions', + content={ + 'included-fbv': { + 'list': coreapi.Link(url='/included-fbv/', action='get') + } + } + ) + + assert len(schema.data) == 1 + assert 'included-fbv' in schema.data + assert schema == expected + + def test_endpoint_enumerator_excludes_correctly(self): + """It is responsibility of EndpointEnumerator to exclude views""" + inspector = EndpointEnumerator(self.patterns) + endpoints = inspector.get_api_endpoints() + + assert len(endpoints) == 1 + path, method, callback = endpoints[0] + assert path == '/included-fbv/' + + def test_should_include_endpoint_excludes_correctly(self): + """This is the specific method that should handle the exclusion""" + inspector = EndpointEnumerator(self.patterns) + + # Not pretty. Mimics internals of EndpointEnumerator to put should_include_endpoint under test + pairs = [(inspector.get_path_from_regex(pattern.regex.pattern), pattern.callback) + for pattern in self.patterns] + + should_include = [ + inspector.should_include_endpoint(*pair) for pair in pairs + ] + + expected = [False, False, True] + + assert should_include == expected + + def test_deprecations(self): + with pytest.warns(PendingDeprecationWarning) as record: + @api_view(["GET"], exclude_from_schema=True) + def view(request): + pass + + assert len(record) == 1 + assert str(record[0].message) == ( + "The `exclude_from_schema` argument to `api_view` is pending " + "deprecation. Use the `schema` decorator instead, passing `None`." + ) + + class OldFashionedExcludedView(APIView): + exclude_from_schema = True + + def get(self, request, *args, **kwargs): + pass + + patterns = [ + url('^excluded-old-fashioned/$', OldFashionedExcludedView.as_view()), + ] + + inspector = EndpointEnumerator(patterns) + with pytest.warns(PendingDeprecationWarning) as record: + inspector.get_api_endpoints() + + assert len(record) == 1 + assert str(record[0].message) == ( + "The `OldFashionedExcludedView.exclude_from_schema` attribute is " + "pending deprecation. Set `schema = None` instead." + )