From e5e6329a222def3b0745f90fc55ee36de95ada83 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 29 Aug 2014 11:29:26 +0100 Subject: [PATCH] Remove `pk_url_field`, `slug_url_field`, `slug_field`. Closes #1773. --- rest_framework/generics.py | 36 ++--------- rest_framework/mixins.py | 36 +++-------- rest_framework/relations.py | 117 ++---------------------------------- 3 files changed, 15 insertions(+), 174 deletions(-) diff --git a/rest_framework/generics.py b/rest_framework/generics.py index cb8061b75..e21dc5c7b 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -76,13 +76,6 @@ class GenericAPIView(views.APIView): model_serializer_class = api_settings.DEFAULT_MODEL_SERIALIZER_CLASS paginator_class = Paginator - ###################################### - # These are pending deprecation... - - pk_url_kwarg = 'pk' - slug_url_kwarg = 'slug' - slug_field = 'slug' - def get_serializer_context(self): """ Extra context provided to the serializer class. @@ -270,7 +263,7 @@ class GenericAPIView(views.APIView): error_format = "'%s' must define 'queryset' or 'model'" raise ImproperlyConfigured(error_format % self.__class__.__name__) - def get_object(self, queryset=None): + def get_object(self): """ Returns the object the view is displaying. @@ -278,36 +271,14 @@ class GenericAPIView(views.APIView): queryset lookups. Eg if objects are referenced using multiple keyword arguments in the url conf. """ - # Determine the base queryset to use. - if queryset is None: - queryset = self.filter_queryset(self.get_queryset()) - else: - pass # Deprecation warning + queryset = self.filter_queryset(self.get_queryset()) # Perform the lookup filtering. # Note that `pk` and `slug` are deprecated styles of lookup filtering. lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field lookup = self.kwargs.get(lookup_url_kwarg, None) - pk = self.kwargs.get(self.pk_url_kwarg, None) - slug = self.kwargs.get(self.slug_url_kwarg, None) - if lookup is not None: - filter_kwargs = {self.lookup_field: lookup} - elif pk is not None and self.lookup_field == 'pk': - warnings.warn( - 'The `pk_url_kwarg` attribute is deprecated. ' - 'Use the `lookup_field` attribute instead', - DeprecationWarning - ) - filter_kwargs = {'pk': pk} - elif slug is not None and self.lookup_field == 'pk': - warnings.warn( - 'The `slug_url_kwarg` attribute is deprecated. ' - 'Use the `lookup_field` attribute instead', - DeprecationWarning - ) - filter_kwargs = {self.slug_field: slug} - else: + if lookup is None: raise ImproperlyConfigured( 'Expected view %s to be called with a URL keyword argument ' 'named "%s". Fix your URL conf, or set the `.lookup_field` ' @@ -315,6 +286,7 @@ class GenericAPIView(views.APIView): (self.__class__.__name__, self.lookup_field) ) + filter_kwargs = {self.lookup_field: lookup} obj = get_object_or_404(queryset, **filter_kwargs) # May raise a permission denied diff --git a/rest_framework/mixins.py b/rest_framework/mixins.py index dc4c9f353..ac59d9795 100644 --- a/rest_framework/mixins.py +++ b/rest_framework/mixins.py @@ -12,10 +12,9 @@ from rest_framework import status from rest_framework.response import Response from rest_framework.request import clone_request from rest_framework.settings import api_settings -import warnings -def _get_validation_exclusions(obj, pk=None, slug_field=None, lookup_field=None): +def _get_validation_exclusions(obj, lookup_field=None): """ Given a model instance, and an optional pk and slug field, return the full list of all other field names on that model. @@ -23,23 +22,13 @@ def _get_validation_exclusions(obj, pk=None, slug_field=None, lookup_field=None) For use when performing full_clean on a model instance, so we only clean the required fields. """ - include = [] - - if pk: - # Deprecated + if lookup_field == 'pk': pk_field = obj._meta.pk while pk_field.rel: pk_field = pk_field.rel.to._meta.pk - include.append(pk_field.name) + lookup_field = pk_field.name - if slug_field: - # Deprecated - include.append(slug_field) - - if lookup_field and lookup_field != 'pk': - include.append(lookup_field) - - return [field.name for field in obj._meta.fields if field.name not in include] + return [field.name for field in obj._meta.fields if field.name != lookup_field] class CreateModelMixin(object): @@ -146,26 +135,15 @@ class UpdateModelMixin(object): """ Set any attributes on the object that are implicit in the request. """ - # pk and/or slug attributes are implicit in the URL. lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field - lookup = self.kwargs.get(lookup_url_kwarg, None) - pk = self.kwargs.get(self.pk_url_kwarg, None) - slug = self.kwargs.get(self.slug_url_kwarg, None) - slug_field = slug and self.slug_field or None + lookup_value = self.kwargs[lookup_url_kwarg] - if lookup: - setattr(obj, self.lookup_field, lookup) - - if pk: - setattr(obj, 'pk', pk) - - if slug: - setattr(obj, slug_field, slug) + setattr(obj, self.lookup_field, lookup_value) # Ensure we clean the attributes so that we don't eg return integer # pk using a string representation, as provided by the url conf kwarg. if hasattr(obj, 'full_clean'): - exclude = _get_validation_exclusions(obj, pk, slug_field, self.lookup_field) + exclude = _get_validation_exclusions(obj, self.lookup_field) obj.full_clean(exclude) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 1acbdce26..56870b408 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -16,7 +16,6 @@ from rest_framework.fields import Field, WritableField, get_component, is_simple from rest_framework.reverse import reverse from rest_framework.compat import urlparse from rest_framework.compat import smart_text -import warnings # Relational fields @@ -320,11 +319,6 @@ class HyperlinkedRelatedField(RelatedField): 'incorrect_type': _('Incorrect type. Expected url string, received %s.'), } - # These are all deprecated - pk_url_kwarg = 'pk' - slug_field = 'slug' - slug_url_kwarg = None # Defaults to same as `slug_field` unless overridden - def __init__(self, *args, **kwargs): try: self.view_name = kwargs.pop('view_name') @@ -334,22 +328,6 @@ class HyperlinkedRelatedField(RelatedField): self.lookup_field = kwargs.pop('lookup_field', self.lookup_field) self.format = kwargs.pop('format', None) - # These are deprecated - if 'pk_url_kwarg' in kwargs: - msg = 'pk_url_kwarg is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - if 'slug_url_kwarg' in kwargs: - msg = 'slug_url_kwarg is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - if 'slug_field' in kwargs: - msg = 'slug_field is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - - self.pk_url_kwarg = kwargs.pop('pk_url_kwarg', self.pk_url_kwarg) - self.slug_field = kwargs.pop('slug_field', self.slug_field) - default_slug_kwarg = self.slug_url_kwarg or self.slug_field - self.slug_url_kwarg = kwargs.pop('slug_url_kwarg', default_slug_kwarg) - super(HyperlinkedRelatedField, self).__init__(*args, **kwargs) def get_url(self, obj, view_name, request, format): @@ -361,39 +339,7 @@ class HyperlinkedRelatedField(RelatedField): """ lookup_field = getattr(obj, self.lookup_field) kwargs = {self.lookup_field: lookup_field} - try: - return reverse(view_name, kwargs=kwargs, request=request, format=format) - except NoReverseMatch: - pass - - if self.pk_url_kwarg != 'pk': - # Only try pk if it has been explicitly set. - # Otherwise, the default `lookup_field = 'pk'` has us covered. - pk = obj.pk - kwargs = {self.pk_url_kwarg: pk} - try: - return reverse(view_name, kwargs=kwargs, request=request, format=format) - except NoReverseMatch: - pass - - slug = getattr(obj, self.slug_field, None) - if slug is not None: - # Only try slug if it corresponds to an attribute on the object. - kwargs = {self.slug_url_kwarg: slug} - try: - ret = reverse(view_name, kwargs=kwargs, request=request, format=format) - if self.slug_field == 'slug' and self.slug_url_kwarg == 'slug': - # If the lookup succeeds using the default slug params, - # then `slug_field` is being used implicitly, and we - # we need to warn about the pending deprecation. - msg = 'Implicit slug field hyperlinked fields are deprecated.' \ - 'You should set `lookup_field=slug` on the HyperlinkedRelatedField.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - return ret - except NoReverseMatch: - pass - - raise NoReverseMatch() + return reverse(view_name, kwargs=kwargs, request=request, format=format) def get_object(self, queryset, view_name, view_args, view_kwargs): """ @@ -402,19 +348,8 @@ class HyperlinkedRelatedField(RelatedField): Takes the matched URL conf arguments, and the queryset, and should return an object instance, or raise an `ObjectDoesNotExist` exception. """ - lookup = view_kwargs.get(self.lookup_field, None) - pk = view_kwargs.get(self.pk_url_kwarg, None) - slug = view_kwargs.get(self.slug_url_kwarg, None) - - if lookup is not None: - filter_kwargs = {self.lookup_field: lookup} - elif pk is not None: - filter_kwargs = {'pk': pk} - elif slug is not None: - filter_kwargs = {self.slug_field: slug} - else: - raise ObjectDoesNotExist() - + lookup_value = view_kwargs[self.lookup_field] + filter_kwargs = {self.lookup_field: lookup_value} return queryset.get(**filter_kwargs) def to_native(self, obj): @@ -486,11 +421,6 @@ class HyperlinkedIdentityField(Field): lookup_field = 'pk' read_only = True - # These are all deprecated - pk_url_kwarg = 'pk' - slug_field = 'slug' - slug_url_kwarg = None # Defaults to same as `slug_field` unless overridden - def __init__(self, *args, **kwargs): try: self.view_name = kwargs.pop('view_name') @@ -502,22 +432,6 @@ class HyperlinkedIdentityField(Field): lookup_field = kwargs.pop('lookup_field', None) self.lookup_field = lookup_field or self.lookup_field - # These are deprecated - if 'pk_url_kwarg' in kwargs: - msg = 'pk_url_kwarg is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - if 'slug_url_kwarg' in kwargs: - msg = 'slug_url_kwarg is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - if 'slug_field' in kwargs: - msg = 'slug_field is deprecated. Use lookup_field instead.' - warnings.warn(msg, DeprecationWarning, stacklevel=2) - - self.slug_field = kwargs.pop('slug_field', self.slug_field) - default_slug_kwarg = self.slug_url_kwarg or self.slug_field - self.pk_url_kwarg = kwargs.pop('pk_url_kwarg', self.pk_url_kwarg) - self.slug_url_kwarg = kwargs.pop('slug_url_kwarg', default_slug_kwarg) - super(HyperlinkedIdentityField, self).__init__(*args, **kwargs) def field_to_native(self, obj, field_name): @@ -569,27 +483,4 @@ class HyperlinkedIdentityField(Field): if lookup_field is None: return None - try: - return reverse(view_name, kwargs=kwargs, request=request, format=format) - except NoReverseMatch: - pass - - if self.pk_url_kwarg != 'pk': - # Only try pk lookup if it has been explicitly set. - # Otherwise, the default `lookup_field = 'pk'` has us covered. - kwargs = {self.pk_url_kwarg: obj.pk} - try: - return reverse(view_name, kwargs=kwargs, request=request, format=format) - except NoReverseMatch: - pass - - slug = getattr(obj, self.slug_field, None) - if slug: - # Only use slug lookup if a slug field exists on the model - kwargs = {self.slug_url_kwarg: slug} - try: - return reverse(view_name, kwargs=kwargs, request=request, format=format) - except NoReverseMatch: - pass - - raise NoReverseMatch() + return reverse(view_name, kwargs=kwargs, request=request, format=format)