From 089162e6e319a1c35f60398319cf70a13d404fa5 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Wed, 13 May 2020 03:11:26 -0700 Subject: [PATCH 1/9] Fix ModelSerializer unique_together handling for field sources (#7143) * Fix ModelSerializer unique_together field sources Updates ModelSerializer to check for serializer fields that map to the model field sources in the unique_together lists. * Ensure field name ordering consistency --- rest_framework/serializers.py | 51 ++++++++++++++++++++++++++--------- tests/test_validators.py | 43 +++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 8c2486bea..c1cea1e83 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -13,7 +13,7 @@ response content is handled by parsers and renderers. import copy import inspect import traceback -from collections import OrderedDict +from collections import OrderedDict, defaultdict from collections.abc import Mapping from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured @@ -1508,28 +1508,55 @@ class ModelSerializer(Serializer): # which may map onto a model field. Any dotted field name lookups # cannot map to a field, and must be a traversal, so we're not # including those. - field_names = { - field.source for field in self._writable_fields + field_sources = OrderedDict( + (field.field_name, field.source) for field in self._writable_fields if (field.source != '*') and ('.' not in field.source) - } + ) # Special Case: Add read_only fields with defaults. - field_names |= { - field.source for field in self.fields.values() + field_sources.update(OrderedDict( + (field.field_name, field.source) for field in self.fields.values() if (field.read_only) and (field.default != empty) and (field.source != '*') and ('.' not in field.source) - } + )) + + # Invert so we can find the serializer field names that correspond to + # the model field names in the unique_together sets. This also allows + # us to check that multiple fields don't map to the same source. + source_map = defaultdict(list) + for name, source in field_sources.items(): + source_map[source].append(name) # Note that we make sure to check `unique_together` both on the # base model class, but also on any parent classes. validators = [] for parent_class in model_class_inheritance_tree: for unique_together in parent_class._meta.unique_together: - if field_names.issuperset(set(unique_together)): - validator = UniqueTogetherValidator( - queryset=parent_class._default_manager, - fields=unique_together + # Skip if serializer does not map to all unique together sources + if not set(source_map).issuperset(set(unique_together)): + continue + + for source in unique_together: + assert len(source_map[source]) == 1, ( + "Unable to create `UniqueTogetherValidator` for " + "`{model}.{field}` as `{serializer}` has multiple " + "fields ({fields}) that map to this model field. " + "Either remove the extra fields, or override " + "`Meta.validators` with a `UniqueTogetherValidator` " + "using the desired field names." + .format( + model=self.Meta.model.__name__, + serializer=self.__class__.__name__, + field=source, + fields=', '.join(source_map[source]), + ) ) - validators.append(validator) + + field_names = tuple(source_map[f][0] for f in unique_together) + validator = UniqueTogetherValidator( + queryset=parent_class._default_manager, + fields=field_names + ) + validators.append(validator) return validators def get_unique_for_date_validators(self): diff --git a/tests/test_validators.py b/tests/test_validators.py index 21c00073d..4962cf581 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -344,6 +344,49 @@ class TestUniquenessTogetherValidation(TestCase): ] } + def test_default_validator_with_fields_with_source(self): + class TestSerializer(serializers.ModelSerializer): + name = serializers.CharField(source='race_name') + + class Meta: + model = UniquenessTogetherModel + fields = ['name', 'position'] + + serializer = TestSerializer() + expected = dedent(""" + TestSerializer(): + name = CharField(source='race_name') + position = IntegerField() + class Meta: + validators = [] + """) + assert repr(serializer) == expected + + def test_default_validator_with_multiple_fields_with_same_source(self): + class TestSerializer(serializers.ModelSerializer): + name = serializers.CharField(source='race_name') + other_name = serializers.CharField(source='race_name') + + class Meta: + model = UniquenessTogetherModel + fields = ['name', 'other_name', 'position'] + + serializer = TestSerializer(data={ + 'name': 'foo', + 'other_name': 'foo', + 'position': 1, + }) + with pytest.raises(AssertionError) as excinfo: + serializer.is_valid() + + expected = ( + "Unable to create `UniqueTogetherValidator` for " + "`UniquenessTogetherModel.race_name` as `TestSerializer` has " + "multiple fields (name, other_name) that map to this model field. " + "Either remove the extra fields, or override `Meta.validators` " + "with a `UniqueTogetherValidator` using the desired field names.") + assert str(excinfo.value) == expected + def test_allow_explict_override(self): """ Ensure validators can be explicitly removed.. From e888fc11c76d1a671f6f7d3c080915ffe722f3f6 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Wed, 13 May 2020 09:59:04 -0400 Subject: [PATCH 2/9] Merge NullBooleanField with BooleanField(allow_null=True) (#7122) * Make `NullBooleanField` subclass `BooleanField` This removes a lot of the redundancy that was in place becuase we were not doing this. This maintains the `None` initial value that was previously present, as well as disallowing `allow_null` to be passed in. * Remove special case for mapping `NullBooleanField` In newer versions of Django, the `NullBooleanField` is handled the same way as a `BooleanField(null=True)`. Given that we also support that combination, and that our own `NullBooleanField` behaves in the same manner, it makes sense to remove the special casing that exists for it. * Add test for BooleanField(null=True, choices) * Remove special case for NullBooleanField * Adjust mapping tests for NullBooleanField * Fixed linting error * Raise deprecation warning when NullBooleanField is used * Fix linting issue in imports --- rest_framework/__init__.py | 4 ++ rest_framework/fields.py | 55 ++++++--------------------- rest_framework/serializers.py | 2 +- rest_framework/utils/field_mapping.py | 2 +- tests/test_model_serializer.py | 23 ++++++++++- 5 files changed, 40 insertions(+), 46 deletions(-) diff --git a/rest_framework/__init__.py b/rest_framework/__init__.py index 8f2bc4466..84e9a8b86 100644 --- a/rest_framework/__init__.py +++ b/rest_framework/__init__.py @@ -31,3 +31,7 @@ class RemovedInDRF313Warning(DeprecationWarning): class RemovedInDRF314Warning(PendingDeprecationWarning): pass + + +class RemovedInDRF314Warning(PendingDeprecationWarning): + pass diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 958bebeef..0447eb74d 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -30,7 +30,9 @@ from django.utils.timezone import utc from django.utils.translation import gettext_lazy as _ from pytz.exceptions import InvalidTimeError -from rest_framework import ISO_8601, RemovedInDRF313Warning +from rest_framework import ( + ISO_8601, RemovedInDRF313Warning, RemovedInDRF314Warning +) from rest_framework.compat import ProhibitNullCharactersValidator from rest_framework.exceptions import ErrorDetail, ValidationError from rest_framework.settings import api_settings @@ -740,55 +742,22 @@ class BooleanField(Field): return bool(value) -class NullBooleanField(Field): - default_error_messages = { - 'invalid': _('Must be a valid boolean.') - } +class NullBooleanField(BooleanField): initial = None - TRUE_VALUES = { - 't', 'T', - 'y', 'Y', 'yes', 'YES', - 'true', 'True', 'TRUE', - 'on', 'On', 'ON', - '1', 1, - True - } - FALSE_VALUES = { - 'f', 'F', - 'n', 'N', 'no', 'NO', - 'false', 'False', 'FALSE', - 'off', 'Off', 'OFF', - '0', 0, 0.0, - False - } - NULL_VALUES = {'null', 'Null', 'NULL', '', None} def __init__(self, **kwargs): + warnings.warn( + "The `NullBooleanField` is deprecated and will be removed starting " + "with 3.14. Instead use the `BooleanField` field and set " + "`null=True` which does the same thing.", + RemovedInDRF314Warning, stacklevel=2 + ) + assert 'allow_null' not in kwargs, '`allow_null` is not a valid option.' kwargs['allow_null'] = True + super().__init__(**kwargs) - def to_internal_value(self, data): - try: - if data in self.TRUE_VALUES: - return True - elif data in self.FALSE_VALUES: - return False - elif data in self.NULL_VALUES: - return None - except TypeError: # Input is an unhashable type - pass - self.fail('invalid', input=data) - - def to_representation(self, value): - if value in self.NULL_VALUES: - return None - if value in self.TRUE_VALUES: - return True - elif value in self.FALSE_VALUES: - return False - return bool(value) - # String types... diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c1cea1e83..cfb54de13 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -868,7 +868,7 @@ class ModelSerializer(Serializer): models.FloatField: FloatField, models.ImageField: ImageField, models.IntegerField: IntegerField, - models.NullBooleanField: NullBooleanField, + models.NullBooleanField: BooleanField, models.PositiveIntegerField: IntegerField, models.PositiveSmallIntegerField: IntegerField, models.SlugField: SlugField, diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index a25880d0f..ed270be5e 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -104,7 +104,7 @@ def get_field_kwargs(field_name, model_field): if model_field.has_default() or model_field.blank or model_field.null: kwargs['required'] = False - if model_field.null and not isinstance(model_field, models.NullBooleanField): + if model_field.null: kwargs['allow_null'] = True if model_field.blank and (isinstance(model_field, (models.CharField, models.TextField))): diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 0de628dc8..51b8f2e22 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -182,7 +182,7 @@ class TestRegularFieldMappings(TestCase): email_field = EmailField(max_length=100) float_field = FloatField() integer_field = IntegerField() - null_boolean_field = NullBooleanField(required=False) + null_boolean_field = BooleanField(allow_null=True, required=False) positive_integer_field = IntegerField() positive_small_integer_field = IntegerField() slug_field = SlugField(allow_unicode=False, max_length=100) @@ -236,6 +236,27 @@ class TestRegularFieldMappings(TestCase): self.assertEqual(repr(NullableBooleanSerializer()), expected) + def test_nullable_boolean_field_choices(self): + class NullableBooleanChoicesModel(models.Model): + CHECKLIST_OPTIONS = ( + (None, 'Unknown'), + (True, 'Yes'), + (False, 'No'), + ) + + field = models.BooleanField(null=True, choices=CHECKLIST_OPTIONS) + + class NullableBooleanChoicesSerializer(serializers.ModelSerializer): + class Meta: + model = NullableBooleanChoicesModel + fields = ['field'] + + serializer = NullableBooleanChoicesSerializer(data=dict( + field=None, + )) + self.assertTrue(serializer.is_valid()) + self.assertEqual(serializer.errors, {}) + def test_method_field(self): """ Properties and methods on the model should be allowed as `Meta.fields` From 8bb9a37f4b680a9efb43996705df885dc5b3802e Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 13 May 2020 20:41:53 +0200 Subject: [PATCH 3/9] Removed duplicated class RemovedInDRF314Warning. Added accidently in e888fc11c76d1a671f6f7d3c080915ffe722f3f6 Co-authored-by: Jair Henrique --- rest_framework/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rest_framework/__init__.py b/rest_framework/__init__.py index 84e9a8b86..8f2bc4466 100644 --- a/rest_framework/__init__.py +++ b/rest_framework/__init__.py @@ -31,7 +31,3 @@ class RemovedInDRF313Warning(DeprecationWarning): class RemovedInDRF314Warning(PendingDeprecationWarning): pass - - -class RemovedInDRF314Warning(PendingDeprecationWarning): - pass From fccfdd21c078579bb0029db7289d5e19d58201e8 Mon Sep 17 00:00:00 2001 From: johnthagen Date: Wed, 13 May 2020 20:54:46 -0400 Subject: [PATCH 4/9] Remove object inheritance in docs (#7332) --- docs/api-guide/fields.md | 2 +- docs/api-guide/generic-views.md | 2 +- docs/api-guide/serializers.md | 2 +- docs/api-guide/validators.md | 2 +- rest_framework/schemas/generators.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/api-guide/fields.md b/docs/api-guide/fields.md index 65c83b78e..b2bdd50c8 100644 --- a/docs/api-guide/fields.md +++ b/docs/api-guide/fields.md @@ -603,7 +603,7 @@ The `to_internal_value()` method is called to restore a primitive datatype into Let's look at an example of serializing a class that represents an RGB color value: - class Color(object): + class Color: """ A color represented in the RGB colorspace. """ diff --git a/docs/api-guide/generic-views.md b/docs/api-guide/generic-views.md index a256eb2d9..4ff549f07 100644 --- a/docs/api-guide/generic-views.md +++ b/docs/api-guide/generic-views.md @@ -319,7 +319,7 @@ Often you'll want to use the existing generic views, but use some slightly custo For example, if you need to lookup objects based on multiple fields in the URL conf, you could create a mixin class like the following: - class MultipleFieldLookupMixin(object): + class MultipleFieldLookupMixin: """ Apply this mixin to any view or viewset to get multiple field filtering based on a `lookup_fields` attribute, instead of the default single field filtering. diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index 87d3d4056..4f566ff59 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -21,7 +21,7 @@ Let's start by creating a simple object we can use for example purposes: from datetime import datetime - class Comment(object): + class Comment: def __init__(self, email, content, created=None): self.email = email self.content = content diff --git a/docs/api-guide/validators.md b/docs/api-guide/validators.md index 009cd2468..4451489d4 100644 --- a/docs/api-guide/validators.md +++ b/docs/api-guide/validators.md @@ -282,7 +282,7 @@ to your `Serializer` subclass. This is documented in the To write a class-based validator, use the `__call__` method. Class-based validators are useful as they allow you to parameterize and reuse behavior. - class MultipleOf(object): + class MultipleOf: def __init__(self, base): self.base = base diff --git a/rest_framework/schemas/generators.py b/rest_framework/schemas/generators.py index 4b6d82a14..77502d028 100644 --- a/rest_framework/schemas/generators.py +++ b/rest_framework/schemas/generators.py @@ -143,7 +143,7 @@ class EndpointEnumerator: return [method for method in methods if method not in ('OPTIONS', 'HEAD')] -class BaseSchemaGenerator(object): +class BaseSchemaGenerator: endpoint_inspector_cls = EndpointEnumerator # 'pk' isn't great as an externally exposed name for an identifier, From aed74961ba03e3e6f53c468353f4e255eb788555 Mon Sep 17 00:00:00 2001 From: Jair Henrique Date: Thu, 14 May 2020 04:24:09 -0300 Subject: [PATCH 5/9] Remove compat for ProhibitNullCharactersValidator (#7333) --- rest_framework/compat.py | 5 ----- rest_framework/fields.py | 8 +++----- tests/test_fields.py | 6 ++---- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index df100966b..1de23bfaa 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -19,11 +19,6 @@ except ImportError: RegexURLResolver as URLResolver, ) -try: - from django.core.validators import ProhibitNullCharactersValidator # noqa -except ImportError: - ProhibitNullCharactersValidator = None - def get_original_route(urlpattern): """ diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 0447eb74d..da2dd54be 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -14,7 +14,8 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import ( EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator, - MinValueValidator, RegexValidator, URLValidator, ip_address_validators + MinValueValidator, ProhibitNullCharactersValidator, RegexValidator, + URLValidator, ip_address_validators ) from django.forms import FilePathField as DjangoFilePathField from django.forms import ImageField as DjangoImageField @@ -33,7 +34,6 @@ from pytz.exceptions import InvalidTimeError from rest_framework import ( ISO_8601, RemovedInDRF313Warning, RemovedInDRF314Warning ) -from rest_framework.compat import ProhibitNullCharactersValidator from rest_framework.exceptions import ErrorDetail, ValidationError from rest_framework.settings import api_settings from rest_framework.utils import html, humanize_datetime, json, representation @@ -785,9 +785,7 @@ class CharField(Field): self.validators.append( MinLengthValidator(self.min_length, message=message)) - # ProhibitNullCharactersValidator is None on Django < 2.0 - if ProhibitNullCharactersValidator is not None: - self.validators.append(ProhibitNullCharactersValidator()) + self.validators.append(ProhibitNullCharactersValidator()) self.validators.append(ProhibitSurrogateCharactersValidator()) def run_validation(self, data=empty): diff --git a/tests/test_fields.py b/tests/test_fields.py index a4b78fd51..b1ad1dc66 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -13,7 +13,6 @@ from django.utils.timezone import activate, deactivate, override, utc import rest_framework from rest_framework import exceptions, serializers -from rest_framework.compat import ProhibitNullCharactersValidator from rest_framework.fields import ( BuiltinSignatureError, DjangoImageField, is_simple_callable ) @@ -747,7 +746,6 @@ class TestCharField(FieldValues): field.run_validation(' ') assert exc_info.value.detail == ['This field may not be blank.'] - @pytest.mark.skipif(ProhibitNullCharactersValidator is None, reason="Skipped on Django < 2.0") def test_null_bytes(self): field = serializers.CharField() @@ -762,8 +760,8 @@ class TestCharField(FieldValues): field = serializers.CharField() for code_point, expected_message in ( - (0xD800, 'Surrogate characters are not allowed: U+D800.'), - (0xDFFF, 'Surrogate characters are not allowed: U+DFFF.'), + (0xD800, 'Surrogate characters are not allowed: U+D800.'), + (0xDFFF, 'Surrogate characters are not allowed: U+DFFF.'), ): with pytest.raises(serializers.ValidationError) as exc_info: field.run_validation(chr(code_point)) From b83e9121f39822a873b45f2c42c88c7a59c64f87 Mon Sep 17 00:00:00 2001 From: Jair Henrique Date: Thu, 14 May 2020 10:48:14 -0300 Subject: [PATCH 6/9] Remove compat urls for Django < 2.0 (#7335) --- rest_framework/compat.py | 10 ---------- rest_framework/urlpatterns.py | 3 ++- tests/schemas/test_coreapi.py | 3 ++- tests/test_urlpatterns.py | 4 ++-- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 1de23bfaa..acace3467 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -212,16 +212,6 @@ else: return False -# Django 1.x url routing syntax. Remove when dropping Django 1.11 support. -try: - from django.urls import include, path, re_path, register_converter # noqa -except ImportError: - from django.conf.urls import include, url # noqa - path = None - register_converter = None - re_path = url - - # `separators` argument to `json.dumps()` differs between 2.x and 3.x # See: https://bugs.python.org/issue22767 SHORT_SEPARATORS = (',', ':') diff --git a/rest_framework/urlpatterns.py b/rest_framework/urlpatterns.py index 831d344dd..9c82e1633 100644 --- a/rest_framework/urlpatterns.py +++ b/rest_framework/urlpatterns.py @@ -1,7 +1,8 @@ from django.conf.urls import include, url +from django.urls import path, register_converter from rest_framework.compat import ( - URLResolver, get_regex_pattern, is_route_pattern, path, register_converter + URLResolver, get_regex_pattern, is_route_pattern ) from rest_framework.settings import api_settings diff --git a/tests/schemas/test_coreapi.py b/tests/schemas/test_coreapi.py index a634d6968..77e18a9a1 100644 --- a/tests/schemas/test_coreapi.py +++ b/tests/schemas/test_coreapi.py @@ -5,11 +5,12 @@ from django.conf.urls import include, url from django.core.exceptions import PermissionDenied from django.http import Http404 from django.test import TestCase, override_settings +from django.urls import path from rest_framework import ( filters, generics, pagination, permissions, serializers ) -from rest_framework.compat import coreapi, coreschema, get_regex_pattern, path +from rest_framework.compat import coreapi, coreschema, get_regex_pattern from rest_framework.decorators import action, api_view, schema from rest_framework.request import Request from rest_framework.routers import DefaultRouter, SimpleRouter diff --git a/tests/test_urlpatterns.py b/tests/test_urlpatterns.py index 25cc0032e..51d269535 100644 --- a/tests/test_urlpatterns.py +++ b/tests/test_urlpatterns.py @@ -3,9 +3,9 @@ from collections import namedtuple from django.conf.urls import include, url from django.test import TestCase -from django.urls import Resolver404 +from django.urls import Resolver404, path, re_path -from rest_framework.compat import make_url_resolver, path, re_path +from rest_framework.compat import make_url_resolver from rest_framework.test import APIRequestFactory from rest_framework.urlpatterns import format_suffix_patterns From 65add6679d5eebe5c8baadb02b4c105da388e0e5 Mon Sep 17 00:00:00 2001 From: Jair Henrique Date: Thu, 14 May 2020 10:49:04 -0300 Subject: [PATCH 7/9] Remove unnecessary test skips (#7336) --- rest_framework/compat.py | 6 ----- tests/test_permissions.py | 46 +++++++++++++++++---------------------- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index acace3467..ed1f43b62 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -2,8 +2,6 @@ The `compat` module provides support for backwards compatibility with older versions of Django/Python, and compatibility wrappers around optional packages. """ -import sys - from django.conf import settings from django.views.generic import View @@ -217,7 +215,3 @@ else: SHORT_SEPARATORS = (',', ':') LONG_SEPARATORS = (', ', ': ') INDENT_SEPARATORS = (',', ': ') - - -# Version Constants. -PY36 = sys.version_info >= (3, 6) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index d445f271d..232c72dd2 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -3,7 +3,6 @@ import unittest 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 @@ -14,7 +13,6 @@ from rest_framework import ( HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers, status, views ) -from rest_framework.compat import PY36 from rest_framework.routers import DefaultRouter from rest_framework.test import APIRequestFactory from tests.models import BasicModel @@ -607,7 +605,6 @@ class PermissionsCompositionTests(TestCase): ) assert composed_perm().has_permission(request, None) is True - @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") def test_or_lazyness(self): request = factory.get('/1', format='json') request.user = AnonymousUser() @@ -616,19 +613,18 @@ class PermissionsCompositionTests(TestCase): with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: composed_perm = (permissions.AllowAny | permissions.IsAuthenticated) hasperm = composed_perm().has_permission(request, None) - self.assertIs(hasperm, True) - mock_allow.assert_called_once() + assert hasperm is True + assert mock_allow.call_count == 1 mock_deny.assert_not_called() with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: composed_perm = (permissions.IsAuthenticated | permissions.AllowAny) hasperm = composed_perm().has_permission(request, None) - self.assertIs(hasperm, True) - mock_deny.assert_called_once() - mock_allow.assert_called_once() + assert hasperm is True + assert mock_deny.call_count == 1 + assert mock_allow.call_count == 1 - @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") def test_object_or_lazyness(self): request = factory.get('/1', format='json') request.user = AnonymousUser() @@ -637,19 +633,18 @@ class PermissionsCompositionTests(TestCase): with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: composed_perm = (permissions.AllowAny | permissions.IsAuthenticated) hasperm = composed_perm().has_object_permission(request, None, None) - self.assertIs(hasperm, True) - mock_allow.assert_called_once() + assert hasperm is True + assert mock_allow.call_count == 1 mock_deny.assert_not_called() with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: composed_perm = (permissions.IsAuthenticated | permissions.AllowAny) hasperm = composed_perm().has_object_permission(request, None, None) - self.assertIs(hasperm, True) - mock_deny.assert_called_once() - mock_allow.assert_called_once() + assert hasperm is True + assert mock_deny.call_count == 1 + assert mock_allow.call_count == 1 - @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") def test_and_lazyness(self): request = factory.get('/1', format='json') request.user = AnonymousUser() @@ -658,19 +653,18 @@ class PermissionsCompositionTests(TestCase): with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: composed_perm = (permissions.AllowAny & permissions.IsAuthenticated) hasperm = composed_perm().has_permission(request, None) - self.assertIs(hasperm, False) - mock_allow.assert_called_once() - mock_deny.assert_called_once() + assert hasperm is False + assert mock_allow.call_count == 1 + assert mock_deny.call_count == 1 with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: composed_perm = (permissions.IsAuthenticated & permissions.AllowAny) hasperm = composed_perm().has_permission(request, None) - self.assertIs(hasperm, False) + assert hasperm is False + assert mock_deny.call_count == 1 mock_allow.assert_not_called() - mock_deny.assert_called_once() - @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") def test_object_and_lazyness(self): request = factory.get('/1', format='json') request.user = AnonymousUser() @@ -679,14 +673,14 @@ class PermissionsCompositionTests(TestCase): with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: composed_perm = (permissions.AllowAny & permissions.IsAuthenticated) hasperm = composed_perm().has_object_permission(request, None, None) - self.assertIs(hasperm, False) - mock_allow.assert_called_once() - mock_deny.assert_called_once() + assert hasperm is False + assert mock_allow.call_count == 1 + assert mock_deny.call_count == 1 with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: composed_perm = (permissions.IsAuthenticated & permissions.AllowAny) hasperm = composed_perm().has_object_permission(request, None, None) - self.assertIs(hasperm, False) + assert hasperm is False + assert mock_deny.call_count == 1 mock_allow.assert_not_called() - mock_deny.assert_called_once() From bb795674f86828fc5f15d6d61501cc781811e053 Mon Sep 17 00:00:00 2001 From: Jair Henrique Date: Thu, 14 May 2020 15:31:38 -0300 Subject: [PATCH 8/9] Drop all compat support to Django < 2 urls (#7337) --- rest_framework/compat.py | 59 ---------------------------- rest_framework/schemas/generators.py | 4 +- rest_framework/urlpatterns.py | 14 +++---- tests/schemas/test_coreapi.py | 4 +- tests/test_filters.py | 2 - tests/test_routers.py | 13 +++--- tests/test_urlpatterns.py | 6 +-- 7 files changed, 18 insertions(+), 84 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index ed1f43b62..611068a62 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -5,65 +5,6 @@ versions of Django/Python, and compatibility wrappers around optional packages. from django.conf import settings from django.views.generic import View -try: - from django.urls import ( # noqa - URLPattern, - URLResolver, - ) -except ImportError: - # Will be removed in Django 2.0 - from django.urls import ( # noqa - RegexURLPattern as URLPattern, - RegexURLResolver as URLResolver, - ) - - -def get_original_route(urlpattern): - """ - Get the original route/regex that was typed in by the user into the path(), re_path() or url() directive. This - is in contrast with get_regex_pattern below, which for RoutePattern returns the raw regex generated from the path(). - """ - if hasattr(urlpattern, 'pattern'): - # Django 2.0 - return str(urlpattern.pattern) - else: - # Django < 2.0 - return urlpattern.regex.pattern - - -def get_regex_pattern(urlpattern): - """ - Get the raw regex out of the urlpattern's RegexPattern or RoutePattern. This is always a regular expression, - unlike get_original_route above. - """ - if hasattr(urlpattern, 'pattern'): - # Django 2.0 - return urlpattern.pattern.regex.pattern - else: - # Django < 2.0 - return urlpattern.regex.pattern - - -def is_route_pattern(urlpattern): - if hasattr(urlpattern, 'pattern'): - # Django 2.0 - from django.urls.resolvers import RoutePattern - return isinstance(urlpattern.pattern, RoutePattern) - else: - # Django < 2.0 - return False - - -def make_url_resolver(regex, urlpatterns): - try: - # Django 2.0 - from django.urls.resolvers import RegexPattern - return URLResolver(RegexPattern(regex), urlpatterns) - - except ImportError: - # Django < 2.0 - return URLResolver(regex, urlpatterns) - def unicode_http_header(value): # Coerce HTTP header value to unicode. diff --git a/rest_framework/schemas/generators.py b/rest_framework/schemas/generators.py index 77502d028..d3c6446aa 100644 --- a/rest_framework/schemas/generators.py +++ b/rest_framework/schemas/generators.py @@ -10,9 +10,9 @@ from django.conf import settings from django.contrib.admindocs.views import simplify_regex from django.core.exceptions import PermissionDenied from django.http import Http404 +from django.urls import URLPattern, URLResolver from rest_framework import exceptions -from rest_framework.compat import URLPattern, URLResolver, get_original_route from rest_framework.request import clone_request from rest_framework.settings import api_settings from rest_framework.utils.model_meta import _get_pk @@ -79,7 +79,7 @@ class EndpointEnumerator: api_endpoints = [] for pattern in patterns: - path_regex = prefix + get_original_route(pattern) + path_regex = prefix + str(pattern.pattern) if isinstance(pattern, URLPattern): path = self.get_path_from_regex(path_regex) callback = pattern.callback diff --git a/rest_framework/urlpatterns.py b/rest_framework/urlpatterns.py index 9c82e1633..5b0bb4440 100644 --- a/rest_framework/urlpatterns.py +++ b/rest_framework/urlpatterns.py @@ -1,9 +1,7 @@ from django.conf.urls import include, url -from django.urls import path, register_converter +from django.urls import URLResolver, path, register_converter +from django.urls.resolvers import RoutePattern -from rest_framework.compat import ( - URLResolver, get_regex_pattern, is_route_pattern -) from rest_framework.settings import api_settings @@ -38,7 +36,7 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_r for urlpattern in urlpatterns: if isinstance(urlpattern, URLResolver): # Set of included URL patterns - regex = get_regex_pattern(urlpattern) + regex = urlpattern.pattern.regex.pattern namespace = urlpattern.namespace app_name = urlpattern.app_name kwargs = urlpattern.default_kwargs @@ -49,7 +47,7 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_r suffix_route) # if the original pattern was a RoutePattern we need to preserve it - if is_route_pattern(urlpattern): + if isinstance(urlpattern.pattern, RoutePattern): assert path is not None route = str(urlpattern.pattern) new_pattern = path(route, include((patterns, app_name), namespace), kwargs) @@ -59,7 +57,7 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_r ret.append(new_pattern) else: # Regular URL pattern - regex = get_regex_pattern(urlpattern).rstrip('$').rstrip('/') + suffix_pattern + regex = urlpattern.pattern.regex.pattern.rstrip('$').rstrip('/') + suffix_pattern view = urlpattern.callback kwargs = urlpattern.default_args name = urlpattern.name @@ -68,7 +66,7 @@ def apply_suffix_patterns(urlpatterns, suffix_pattern, suffix_required, suffix_r ret.append(urlpattern) # if the original pattern was a RoutePattern we need to preserve it - if is_route_pattern(urlpattern): + if isinstance(urlpattern.pattern, RoutePattern): assert path is not None assert suffix_route is not None route = str(urlpattern.pattern).rstrip('$').rstrip('/') + suffix_route diff --git a/tests/schemas/test_coreapi.py b/tests/schemas/test_coreapi.py index 77e18a9a1..403b3b634 100644 --- a/tests/schemas/test_coreapi.py +++ b/tests/schemas/test_coreapi.py @@ -10,7 +10,7 @@ from django.urls import path from rest_framework import ( filters, generics, pagination, permissions, serializers ) -from rest_framework.compat import coreapi, coreschema, get_regex_pattern +from rest_framework.compat import coreapi, coreschema from rest_framework.decorators import action, api_view, schema from rest_framework.request import Request from rest_framework.routers import DefaultRouter, SimpleRouter @@ -1079,7 +1079,7 @@ class SchemaGenerationExclusionTests(TestCase): inspector = EndpointEnumerator(self.patterns) # Not pretty. Mimics internals of EndpointEnumerator to put should_include_endpoint under test - pairs = [(inspector.get_path_from_regex(get_regex_pattern(pattern)), pattern.callback) + pairs = [(inspector.get_path_from_regex(pattern.pattern.regex.pattern), pattern.callback) for pattern in self.patterns] should_include = [ diff --git a/tests/test_filters.py b/tests/test_filters.py index e69537666..567e5f83f 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1,7 +1,6 @@ import datetime from importlib import reload as reload_module -import django import pytest from django.core.exceptions import ImproperlyConfigured from django.db import models @@ -191,7 +190,6 @@ class SearchFilterTests(TestCase): assert terms == ['asdf'] - @pytest.mark.skipif(django.VERSION[:2] < (2, 2), reason="requires django 2.2 or higher") def test_search_field_with_additional_transforms(self): from django.test.utils import register_lookup diff --git a/tests/test_routers.py b/tests/test_routers.py index ff927ff33..007cb4768 100644 --- a/tests/test_routers.py +++ b/tests/test_routers.py @@ -8,7 +8,6 @@ from django.test import TestCase, override_settings from django.urls import resolve, reverse from rest_framework import permissions, serializers, viewsets -from rest_framework.compat import get_regex_pattern from rest_framework.decorators import action from rest_framework.response import Response from rest_framework.routers import DefaultRouter, SimpleRouter @@ -192,8 +191,7 @@ class TestCustomLookupFields(URLPatternsTestCase, TestCase): def test_custom_lookup_field_route(self): detail_route = notes_router.urls[-1] - detail_url_pattern = get_regex_pattern(detail_route) - assert '' in detail_url_pattern + assert '' in detail_route.pattern.regex.pattern def test_retrieve_lookup_field_list_view(self): response = self.client.get('/example/notes/') @@ -229,7 +227,7 @@ class TestLookupValueRegex(TestCase): def test_urls_limited_by_lookup_value_regex(self): expected = ['^notes/$', '^notes/(?P[0-9a-f]{32})/$'] for idx in range(len(expected)): - assert expected[idx] == get_regex_pattern(self.urls[idx]) + assert expected[idx] == self.urls[idx].pattern.regex.pattern @override_settings(ROOT_URLCONF='tests.test_routers') @@ -249,8 +247,7 @@ class TestLookupUrlKwargs(URLPatternsTestCase, TestCase): def test_custom_lookup_url_kwarg_route(self): detail_route = kwarged_notes_router.urls[-1] - detail_url_pattern = get_regex_pattern(detail_route) - assert '^notes/(?P' in detail_url_pattern + assert '^notes/(?P' in detail_route.pattern.regex.pattern def test_retrieve_lookup_url_kwarg_detail_view(self): response = self.client.get('/example2/notes/fo/') @@ -273,7 +270,7 @@ class TestTrailingSlashIncluded(TestCase): def test_urls_have_trailing_slash_by_default(self): expected = ['^notes/$', '^notes/(?P[^/.]+)/$'] for idx in range(len(expected)): - assert expected[idx] == get_regex_pattern(self.urls[idx]) + assert expected[idx] == self.urls[idx].pattern.regex.pattern class TestTrailingSlashRemoved(TestCase): @@ -288,7 +285,7 @@ class TestTrailingSlashRemoved(TestCase): def test_urls_can_have_trailing_slash_removed(self): expected = ['^notes$', '^notes/(?P[^/.]+)$'] for idx in range(len(expected)): - assert expected[idx] == get_regex_pattern(self.urls[idx]) + assert expected[idx] == self.urls[idx].pattern.regex.pattern class TestNameableRoot(TestCase): diff --git a/tests/test_urlpatterns.py b/tests/test_urlpatterns.py index 51d269535..ec19494b0 100644 --- a/tests/test_urlpatterns.py +++ b/tests/test_urlpatterns.py @@ -3,9 +3,9 @@ from collections import namedtuple from django.conf.urls import include, url from django.test import TestCase -from django.urls import Resolver404, path, re_path +from django.urls import Resolver404, URLResolver, path, re_path +from django.urls.resolvers import RegexPattern -from rest_framework.compat import make_url_resolver from rest_framework.test import APIRequestFactory from rest_framework.urlpatterns import format_suffix_patterns @@ -28,7 +28,7 @@ class FormatSuffixTests(TestCase): urlpatterns = format_suffix_patterns(urlpatterns, allowed=allowed) except Exception: self.fail("Failed to apply `format_suffix_patterns` on the supplied urlpatterns") - resolver = make_url_resolver(r'^/', urlpatterns) + resolver = URLResolver(RegexPattern(r'^/'), urlpatterns) for test_path in test_paths: try: test_path, expected_resolved = test_path From acbd9d8222e763c7f9c7dc2de23c430c702e06d4 Mon Sep 17 00:00:00 2001 From: Asif Saif Uddin Date: Fri, 15 May 2020 13:40:47 +0600 Subject: [PATCH 9/9] django 3.1 alpha on matrix (#7334) * django 3.1 alpha on matrix * django 3.1 alpha on matrix --- .travis.yml | 3 +++ setup.py | 1 + tox.ini | 3 +++ 3 files changed, 7 insertions(+) diff --git a/.travis.yml b/.travis.yml index f1ec689f7..39efaf4fc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,13 +9,16 @@ matrix: - { python: "3.6", env: DJANGO=2.2 } - { python: "3.6", env: DJANGO=3.0 } + - { python: "3.6", env: DJANGO=3.1 } - { python: "3.6", env: DJANGO=master } - { python: "3.7", env: DJANGO=2.2 } - { python: "3.7", env: DJANGO=3.0 } + - { python: "3.7", env: DJANGO=3.1 } - { python: "3.7", env: DJANGO=master } - { python: "3.8", env: DJANGO=3.0 } + - { python: "3.8", env: DJANGO=3.1 } - { python: "3.8", env: DJANGO=master } - { python: "3.8", env: TOXENV=base } diff --git a/setup.py b/setup.py index 99826b4d0..38e680e10 100755 --- a/setup.py +++ b/setup.py @@ -91,6 +91,7 @@ setup( 'Framework :: Django', 'Framework :: Django :: 2.2', 'Framework :: Django :: 3.0', + 'Framework :: Django :: 3.1', 'Intended Audience :: Developers', 'License :: OSI Approved :: BSD License', 'Operating System :: OS Independent', diff --git a/tox.ini b/tox.ini index e5b8b6402..190865f23 100644 --- a/tox.ini +++ b/tox.ini @@ -2,6 +2,7 @@ envlist = {py35,py36,py37}-django22, {py36,py37,py38}-django30, + {py36,py37,py38}-django31, {py36,py37,py38}-djangomaster, base,dist,lint,docs, @@ -9,6 +10,7 @@ envlist = DJANGO = 2.2: django22 3.0: django30 + 3.1: django31 master: djangomaster [testenv] @@ -20,6 +22,7 @@ setenv = deps = django22: Django>=2.2,<3.0 django30: Django>=3.0,<3.1 + django31: Django>=3.1a1,<3.2 djangomaster: https://github.com/django/django/archive/master.tar.gz -rrequirements/requirements-testing.txt -rrequirements/requirements-optionals.txt