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

Allow schema = None. Deprecate exclude_from_schema #5422

Merged
6 changes: 6 additions & 0 deletions docs/api-guide/schemas.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions docs/api-guide/views.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/topics/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ 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.

### 3.6.4

Expand Down
10 changes: 9 additions & 1 deletion rest_framework/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from __future__ import unicode_literals

import types
import warnings

from django.utils import six

Expand Down Expand Up @@ -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 deprecated. "
Copy link
Member

Choose a reason for hiding this comment

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

"is pending deprecation"

"Use the `schema` decorator instead, passing `None`.",
PendingDeprecationWarning
)
WrappedAPIView.exclude_from_schema = exclude_from_schema

return WrappedAPIView.as_view()
return decorator

Expand Down
2 changes: 1 addition & 1 deletion rest_framework/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
14 changes: 12 additions & 2 deletions rest_framework/schemas/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

See schemas.__init__.py for package overview.
"""
import warnings
from collections import OrderedDict
from importlib import import_module

Expand Down Expand Up @@ -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 `APIView.exclude_from_schema` is deprecated. "
"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.

Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/schemas/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions rest_framework/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
97 changes: 96 additions & 1 deletion tests/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -613,3 +616,95 @@ 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):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rpkilby I'm getting console noise in the test run from this. (Well from the call site but...)

tests/test_schemas.py ............/Users/carlton/Documents/Django-Stack/django-rest-framework/rest_framework/decorators.py:83: PendingDeprecationWarning: The `exclude_from_schema` argument to `api_view` is deprecated. Use the `schema` decorator instead, passing `None`.
  PendingDeprecationWarning
/Users/carlton/Documents/Django-Stack/django-rest-framework/rest_framework/schemas/generators.py:156: PendingDeprecationWarning: OldFashjonedExcludedView. The `APIView.exclude_from_schema` is deprecated. Set `schema = None` instead
  warnings.warn(msg, PendingDeprecationWarning)
....
tests/test_serializer.py .........................................

Can you advise on the best way to keep that quiet? Ta!

Copy link
Member

Choose a reason for hiding this comment

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

Not sure we need to test this. Don't believe that we've done so when deprecating other bits of API.

Copy link
Collaborator Author

@carltongibson carltongibson Sep 14, 2017

Choose a reason for hiding this comment

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

warnings.warn(
"The built in 'rest_framework.filters.FilterSet' is deprecated. "
"You should use 'django_filters.rest_framework.FilterSet' instead.",
DeprecationWarning, stacklevel=2
)

and

@unittest.skipUnless(django_filters, 'django-filter not installed')
def test_backend_deprecation(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
view = FilterFieldsRootView.as_view()
request = factory.get('/')
response = view(request).render()
assert response.status_code == status.HTTP_200_OK
assert response.data == self.data
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("'rest_framework.filters.DjangoFilterBackend' is deprecated.", str(w[-1].message))

It's something @rpkilby always puts in. I thought, Why not? 🙂

Copy link
Member

@rpkilby rpkilby Sep 14, 2017

Choose a reason for hiding this comment

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

It looks like pytest.warns isn't recommended for deprecation warnings, as it doesn't capture output.
ach... I completely misread the docs. Either way, the below section sufficiently explains how to test deprecation warnings.

https://docs.pytest.org/en/latest/warnings.html#ensuring-function-triggers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rpkilby OK. Thanks. I read that too but was still getting the noise...

  1. I went with pytest.warns(PendingDeprecationWarning) because (for me) pytest.deprecated_call() failed with DID NOT WARN. No ideas why that wasn't working.
  2. Despite the noise locally for me, this is not appearing in the travis run — so it must be my local setup.

(There is plenty of noise from py.test in travis but I assume that'll disappear with time.)

Lets call this done. Thanks for the input!

@tomchristie I think we're good to go.

Copy link
Member

Choose a reason for hiding this comment

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

Despite the noise locally for me, this is not appearing in the travis run — so it must be my local setup.

No noise when running the tests on my machine, so that would make sense.

with pytest.warns(PendingDeprecationWarning):
@api_view(["GET"], exclude_from_schema=True)
def view(request):
pass

class OldFashjonedExcludedView(APIView):
exclude_from_schema = True

def get(self, request, *args, **kwargs):
pass

patterns = [
url('^excluded-old-fashioned/$', OldFashjonedExcludedView.as_view()),
]

inspector = EndpointEnumerator(patterns)
with pytest.warns(PendingDeprecationWarning):
inspector.get_api_endpoints()