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

Remove 3.10 deprecations #6687

Merged
merged 3 commits into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rest_framework/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
default_app_config = 'rest_framework.apps.RestFrameworkConfig'


class RemovedInDRF310Warning(DeprecationWarning):
class RemovedInDRF311Warning(DeprecationWarning):
pass


class RemovedInDRF311Warning(PendingDeprecationWarning):
class RemovedInDRF312Warning(PendingDeprecationWarning):
pass
7 changes: 0 additions & 7 deletions rest_framework/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,6 @@ def distinct(queryset, base):
requests = None


def is_guardian_installed():
"""
django-guardian is optional and only imported if in INSTALLED_APPS.
"""
return 'guardian' in settings.INSTALLED_APPS


# PATCH method is not implemented by Django
if 'patch' not in View.http_method_names:
View.http_method_names = View.http_method_names + ['patch']
Expand Down
38 changes: 0 additions & 38 deletions rest_framework/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@
used to annotate methods on viewsets that should be included by routers.
"""
import types
import warnings

from django.forms.utils import pretty_name

from rest_framework import RemovedInDRF310Warning
from rest_framework.views import APIView


Expand Down Expand Up @@ -214,39 +212,3 @@ def options(self, func):

def trace(self, func):
return self._map('trace', func)


def detail_route(methods=None, **kwargs):
"""
Used to mark a method on a ViewSet that should be routed for detail requests.
"""
warnings.warn(
"`detail_route` is deprecated and will be removed in 3.10 in favor of "
"`action`, which accepts a `detail` bool. Use `@action(detail=True)` instead.",
RemovedInDRF310Warning, stacklevel=2
)

def decorator(func):
func = action(methods, detail=True, **kwargs)(func)
if 'url_name' not in kwargs:
func.url_name = func.url_path.replace('_', '-')
return func
return decorator


def list_route(methods=None, **kwargs):
"""
Used to mark a method on a ViewSet that should be routed for list requests.
"""
warnings.warn(
"`list_route` is deprecated and will be removed in 3.10 in favor of "
"`action`, which accepts a `detail` bool. Use `@action(detail=False)` instead.",
RemovedInDRF310Warning, stacklevel=2
)

def decorator(func):
func = action(methods, detail=False, **kwargs)(func)
if 'url_name' not in kwargs:
func.url_name = func.url_path.replace('_', '-')
return func
return decorator
44 changes: 1 addition & 43 deletions rest_framework/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
returned by list views.
"""
import operator
import warnings
from functools import reduce

from django.core.exceptions import ImproperlyConfigured
Expand All @@ -14,10 +13,7 @@
from django.utils.encoding import force_text
from django.utils.translation import gettext_lazy as _

from rest_framework import RemovedInDRF310Warning
from rest_framework.compat import (
coreapi, coreschema, distinct, is_guardian_installed
)
from rest_framework.compat import coreapi, coreschema, distinct
from rest_framework.settings import api_settings


Expand Down Expand Up @@ -315,41 +311,3 @@ def get_schema_operation_parameters(self, view):
},
},
]


class DjangoObjectPermissionsFilter(BaseFilterBackend):
"""
A filter backend that limits results to those where the requesting user
has read object level permissions.
"""
def __init__(self):
warnings.warn(
"`DjangoObjectPermissionsFilter` has been deprecated and moved to "
"the 3rd-party django-rest-framework-guardian package.",
RemovedInDRF310Warning, stacklevel=2
)
assert is_guardian_installed(), 'Using DjangoObjectPermissionsFilter, but django-guardian is not installed'

perm_format = '%(app_label)s.view_%(model_name)s'

def filter_queryset(self, request, queryset, view):
# We want to defer this import until run-time, rather than import-time.
# See https://github.com/encode/django-rest-framework/issues/4608
# (Also see #1624 for why we need to make this import explicitly)
from guardian import VERSION as guardian_version
from guardian.shortcuts import get_objects_for_user

extra = {}
user = request.user
model_cls = queryset.model
kwargs = {
'app_label': model_cls._meta.app_label,
'model_name': model_cls._meta.model_name
}
permission = self.perm_format % kwargs
if tuple(guardian_version) >= (1, 3):
# Maintain behavior compatibility with versions prior to 1.3
extra = {'accept_global_perms': False}
else:
extra = {}
return get_objects_for_user(user, permission, queryset, **extra)
26 changes: 1 addition & 25 deletions rest_framework/routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
from django.urls import NoReverseMatch
from django.utils.deprecation import RenameMethodsBase

from rest_framework import (
RemovedInDRF310Warning, RemovedInDRF311Warning, views
)
from rest_framework import RemovedInDRF311Warning, views
from rest_framework.response import Response
from rest_framework.reverse import reverse
from rest_framework.schemas import SchemaGenerator
Expand All @@ -36,28 +34,6 @@
DynamicRoute = namedtuple('DynamicRoute', ['url', 'name', 'detail', 'initkwargs'])


class DynamicDetailRoute:
def __new__(cls, url, name, initkwargs):
warnings.warn(
"`DynamicDetailRoute` is deprecated and will be removed in 3.10 "
"in favor of `DynamicRoute`, which accepts a `detail` boolean. Use "
"`DynamicRoute(url, name, True, initkwargs)` instead.",
RemovedInDRF310Warning, stacklevel=2
)
return DynamicRoute(url, name, True, initkwargs)


class DynamicListRoute:
def __new__(cls, url, name, initkwargs):
warnings.warn(
"`DynamicListRoute` is deprecated and will be removed in 3.10 in "
"favor of `DynamicRoute`, which accepts a `detail` boolean. Use "
"`DynamicRoute(url, name, False, initkwargs)` instead.",
RemovedInDRF310Warning, stacklevel=2
)
return DynamicRoute(url, name, False, initkwargs)


def escape_curly_brackets(url_path):
"""
Double brackets in regex of url_path for escape string formatting
Expand Down
43 changes: 3 additions & 40 deletions tests/test_decorators.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import pytest
from django.test import TestCase

from rest_framework import RemovedInDRF310Warning, status
from rest_framework import status
from rest_framework.authentication import BasicAuthentication
from rest_framework.decorators import (
action, api_view, authentication_classes, detail_route, list_route,
parser_classes, permission_classes, renderer_classes, schema,
throttle_classes
action, api_view, authentication_classes, parser_classes,
permission_classes, renderer_classes, schema, throttle_classes
)
from rest_framework.parsers import JSONParser
from rest_framework.permissions import IsAuthenticated
Expand Down Expand Up @@ -285,39 +284,3 @@ def test_action():
@test_action.mapping.post
def test_action():
raise NotImplementedError

def test_detail_route_deprecation(self):
with pytest.warns(RemovedInDRF310Warning) as record:
@detail_route()
def view(request):
raise NotImplementedError

assert len(record) == 1
assert str(record[0].message) == (
"`detail_route` is deprecated and will be removed in "
"3.10 in favor of `action`, which accepts a `detail` bool. Use "
"`@action(detail=True)` instead."
)

def test_list_route_deprecation(self):
with pytest.warns(RemovedInDRF310Warning) as record:
@list_route()
def view(request):
raise NotImplementedError

assert len(record) == 1
assert str(record[0].message) == (
"`list_route` is deprecated and will be removed in "
"3.10 in favor of `action`, which accepts a `detail` bool. Use "
"`@action(detail=False)` instead."
)

def test_route_url_name_from_path(self):
# pre-3.8 behavior was to base the `url_name` off of the `url_path`
with pytest.warns(RemovedInDRF310Warning):
@list_route(url_path='foo_bar')
def view(request):
raise NotImplementedError

assert view.url_path == 'foo_bar'
assert view.url_name == 'foo-bar'
40 changes: 8 additions & 32 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
import base64
import unittest
import warnings
from unittest import mock

import django
import pytest
from django.conf import settings
from django.contrib.auth.models import AnonymousUser, Group, Permission, User
from django.db import models
from django.test import TestCase
from django.urls import ResolverMatch

from rest_framework import (
HTTP_HEADER_ENCODING, RemovedInDRF310Warning, authentication, generics,
permissions, serializers, status, views
HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers,
status, views
)
from rest_framework.compat import PY36, is_guardian_installed
from rest_framework.filters import DjangoObjectPermissionsFilter
from rest_framework.compat import PY36
from rest_framework.routers import DefaultRouter
from rest_framework.test import APIRequestFactory
from tests.models import BasicModel
Expand Down Expand Up @@ -309,7 +308,7 @@ def get_queryset(self):
get_queryset_object_permissions_view = GetQuerysetObjectPermissionInstanceView.as_view()


@unittest.skipUnless(is_guardian_installed(), 'django-guardian not installed')
@unittest.skipUnless('guardian' in settings.INSTALLED_APPS, 'django-guardian not installed')
class ObjectPermissionsIntegrationTests(TestCase):
"""
Integration tests for the object level permissions API.
Expand Down Expand Up @@ -418,37 +417,14 @@ def test_can_read_get_queryset_permissions(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)

# Read list
def test_django_object_permissions_filter_deprecated(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")
DjangoObjectPermissionsFilter()

message = ("`DjangoObjectPermissionsFilter` has been deprecated and moved "
"to the 3rd-party django-rest-framework-guardian package.")
self.assertEqual(len(w), 1)
self.assertIs(w[-1].category, RemovedInDRF310Warning)
self.assertEqual(str(w[-1].message), message)

# Note: this previously tested `DjangoObjectPermissionsFilter`, which has
# since been moved to a separate package. These now act as sanity checks.
def test_can_read_list_permissions(self):
request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['readonly'])
object_permissions_list_view.cls.filter_backends = (DjangoObjectPermissionsFilter,)
# TODO: remove in version 3.10
with warnings.catch_warnings(record=True):
warnings.simplefilter("always")
response = object_permissions_list_view(request)
response = object_permissions_list_view(request)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data[0].get('id'), 1)

def test_cannot_read_list_permissions(self):
request = factory.get('/', HTTP_AUTHORIZATION=self.credentials['writeonly'])
object_permissions_list_view.cls.filter_backends = (DjangoObjectPermissionsFilter,)
# TODO: remove in version 3.10
with warnings.catch_warnings(record=True):
warnings.simplefilter("always")
response = object_permissions_list_view(request)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(response.data, [])

def test_cannot_method_not_allowed(self):
request = factory.generic('METHOD_NOT_ALLOWED', '/', HTTP_AUTHORIZATION=self.credentials['readonly'])
response = object_permissions_list_view(request)
Expand Down