From 14ae52a24e93063f77c6010269bf9cd3316627fe Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 8 Oct 2014 16:09:37 +0100 Subject: [PATCH] More gradual deprecation --- docs/topics/3.0-announcement.md | 26 +++++----- rest_framework/request.py | 16 ++++++ rest_framework/serializers.py | 71 ++++++++++++++++++++++++++- rest_framework/utils/field_mapping.py | 22 ++++----- rest_framework/utils/model_meta.py | 4 +- 5 files changed, 113 insertions(+), 26 deletions(-) diff --git a/docs/topics/3.0-announcement.md b/docs/topics/3.0-announcement.md index 89817ea5d..6520f2bdd 100644 --- a/docs/topics/3.0-announcement.md +++ b/docs/topics/3.0-announcement.md @@ -8,10 +8,9 @@ See the [Version 3.0 GitHub issue](https://github.com/tomchristie/django-rest-fr The most notable outstanding issues still to be resolved on the `version-3.0` branch are as follows: -* Forms support for serializers and in the browsable API. +* Finish forms support for serializers and in the browsable API. * Optimisations for serializing primary keys. * Refine style of validation errors in some cases, such as validation errors in `ListField`. -* `.transform_()` method on serializers. **Your feedback on the upgrade process and 3.0 changes is hugely important!** @@ -48,12 +47,13 @@ Below is an in-depth guide to the API changes and migration notes for 3.0. #### The `.data` and `.query_params` properties. -The usage of `request.DATA` and `request.FILES` is now discouraged in favor of a single `request.data` attribute that contains *all* the parsed data. +The usage of `request.DATA` and `request.FILES` is now pending deprecation in favor of a single `request.data` attribute that contains *all* the parsed data. -Having separate attributes is reasonable for web applications that only ever parse URL encoded or MultiPart requests, but makes less sense for the general-purpose request parsing that REST framework supports. +Having separate attributes is reasonable for web applications that only ever parse url-encoded or multipart requests, but makes less sense for the general-purpose request parsing that REST framework supports. You may now pass all the request data to a serializer class in a single argument: + # Do this... ExampleSerializer(data=request.data) Instead of passing the files argument separately: @@ -62,7 +62,7 @@ Instead of passing the files argument separately: ExampleSerializer(data=request.DATA, files=request.FILES) -The usage of `request.QUERY_PARAMS` is now discouraged in favor of the lowercased `request.query_params`. +The usage of `request.QUERY_PARAMS` is now pending deprecation in favor of the lowercased `request.query_params`. ## Serializers @@ -73,7 +73,7 @@ Previously the serializers used a two-step object creation, as follows: 1. Validating the data would create an object instance. This instance would be available as `serializer.object`. 2. Calling `serializer.save()` would then save the object instance to the database. -This style is in line with how the `ModelForm` class works in Django, but is problematic for a number of reasons: +This style is in-line with how the `ModelForm` class works in Django, but is problematic for a number of reasons: * Some data, such as many-to-many relationships, cannot be added to the object instance until after it has been saved. This type of data needed to be hidden in some undocumented state on the object instance, or kept as state on the serializer instance so that it could be used when `.save()` is called. * Instantiating model instances directly means that you cannot use model manager classes for instance creation, eg `ExampleModel.objects.create(...)`. Manager classes are an excellent layer at which to enforce business logic and application-level data constraints. @@ -109,7 +109,7 @@ The following example from the tutorial previously used `restore_object()` to ha This would now be split out into two separate methods. - def update(self, instance, validated_data) + def update(self, instance, validated_data): instance.title = validated_data.get('title', instance.title) instance.code = validated_data.get('code', instance.code) instance.linenos = validated_data.get('linenos', instance.linenos) @@ -211,28 +211,30 @@ The `exclude` option on `ModelSerializer` is no longer available. You should use #### The `extra_kwargs` option. -The `read_only_fields` and `write_only_fields` options on `ModelSerializer` have been removed and replaced with a more generic `extra_kwargs`. +The `write_only_fields` option on `ModelSerializer` has been moved to `PendingDeprecation` and replaced with a more generic `extra_kwargs`. class MySerializer(serializer.ModelSerializer): class Meta: model = MyModel fields = ('id', 'email', 'notes', 'is_admin') extra_kwargs = { - 'is_admin': {'read_only': True} + 'is_admin': {'write_only': True} } Alternatively, specify the field explicitly on the serializer class: class MySerializer(serializer.ModelSerializer): - is_admin = serializers.BooleanField(read_only=True) + is_admin = serializers.BooleanField(write_only=True) class Meta: model = MyModel fields = ('id', 'email', 'notes', 'is_admin') +The `read_only_fields` option remains as a convenient shortcut for the more common case. + #### Changes to `HyperlinkedModelSerializer`. -The `view_name` and `lookup_field` options have been removed. They are no longer required, as you can use the `extra_kwargs` argument instead: +The `view_name` and `lookup_field` options have been moved to `PendingDeprecation`. They are no longer required, as you can use the `extra_kwargs` argument instead: class MySerializer(serializer.HyperlinkedModelSerializer): class Meta: @@ -633,7 +635,7 @@ If you need to restore the previous behavior you can include the `AllowPUTAsCrea The generic views now raise `ValidationError` for invalid data. This exception is then dealt with by the exception handler, rather than the view returning a `400 Bad Request` response directly. -This change means that you can now easily cusomize the style of error responses across your entire API, without having to modify any of the generic views. +This change means that you can now easily customize the style of error responses across your entire API, without having to modify any of the generic views. ## The metadata API diff --git a/rest_framework/request.py b/rest_framework/request.py index d80baa705..d43527421 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -18,6 +18,7 @@ from rest_framework import HTTP_HEADER_ENCODING from rest_framework import exceptions from rest_framework.compat import BytesIO from rest_framework.settings import api_settings +import warnings def is_form_media_type(media_type): @@ -209,6 +210,11 @@ class Request(object): """ Synonym for `.query_params`, for backwards compatibility. """ + warnings.warn( + "`request.QUERY_PARAMS` is pending deprecation. Use `request.query_params` instead.", + PendingDeprecationWarning, + stacklevel=1 + ) return self._request.GET @property @@ -225,6 +231,11 @@ class Request(object): Similar to usual behaviour of `request.POST`, except that it handles arbitrary parsers, and also works on methods other than POST (eg PUT). """ + warnings.warn( + "`request.DATA` is pending deprecation. Use `request.data` instead.", + PendingDeprecationWarning, + stacklevel=1 + ) if not _hasattr(self, '_data'): self._load_data_and_files() return self._data @@ -237,6 +248,11 @@ class Request(object): Similar to usual behaviour of `request.FILES`, except that it handles arbitrary parsers, and also works on methods other than POST (eg PUT). """ + warnings.warn( + "`request.FILES` is pending deprecation. Use `request.data` instead.", + PendingDeprecationWarning, + stacklevel=1 + ) if not _hasattr(self, '_files'): self._load_data_and_files() return self._files diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 8513428cf..9fcbcba76 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -25,6 +25,7 @@ from rest_framework.utils.field_mapping import ( from rest_framework.validators import UniqueTogetherValidator import copy import inspect +import warnings # Note: We do the following so that users of the framework can use this style: # @@ -517,12 +518,24 @@ class ModelSerializer(Serializer): depth = getattr(self.Meta, 'depth', 0) extra_kwargs = getattr(self.Meta, 'extra_kwargs', {}) + extra_kwargs = self._include_additional_options(extra_kwargs) + # Retrieve metadata about fields & relationships on the model class. info = model_meta.get_field_info(model) # Use the default set of fields if none is supplied explicitly. if fields is None: fields = self._get_default_field_names(declared_fields, info) + exclude = getattr(self.Meta, 'exclude', None) + if exclude is not None: + warnings.warn( + "The `Meta.exclude` option is pending deprecation. " + "Use the explicit `Meta.fields` instead.", + PendingDeprecationWarning, + stacklevel=3 + ) + for field_name in exclude: + fields.remove(field_name) for field_name in fields: if field_name in declared_fields: @@ -589,13 +602,69 @@ class ModelSerializer(Serializer): ) # Populate any kwargs defined in `Meta.extra_kwargs` - kwargs.update(extra_kwargs.get(field_name, {})) + extras = extra_kwargs.get(field_name, {}) + if extras.get('read_only', False): + for attr in [ + 'required', 'default', 'allow_blank', 'allow_null', + 'min_length', 'max_length', 'min_value', 'max_value', + 'validators' + ]: + kwargs.pop(attr, None) + kwargs.update(extras) # Create the serializer field. ret[field_name] = field_cls(**kwargs) return ret + def _include_additional_options(self, extra_kwargs): + read_only_fields = getattr(self.Meta, 'read_only_fields', None) + if read_only_fields is not None: + for field_name in read_only_fields: + kwargs = extra_kwargs.get(field_name, {}) + kwargs['read_only'] = True + extra_kwargs[field_name] = kwargs + + # These are all pending deprecation. + write_only_fields = getattr(self.Meta, 'write_only_fields', None) + if write_only_fields is not None: + warnings.warn( + "The `Meta.write_only_fields` option is pending deprecation. " + "Use `Meta.extra_kwargs={: {'write_only': True}}` instead.", + PendingDeprecationWarning, + stacklevel=3 + ) + for field_name in write_only_fields: + kwargs = extra_kwargs.get(field_name, {}) + kwargs['write_only'] = True + extra_kwargs[field_name] = kwargs + + view_name = getattr(self.Meta, 'view_name', None) + if view_name is not None: + warnings.warn( + "The `Meta.view_name` option is pending deprecation. " + "Use `Meta.extra_kwargs={'url': {'view_name': ...}}` instead.", + PendingDeprecationWarning, + stacklevel=3 + ) + kwargs = extra_kwargs.get(field_name, {}) + kwargs['view_name'] = view_name + extra_kwargs[api_settings.URL_FIELD_NAME] = kwargs + + lookup_field = getattr(self.Meta, 'lookup_field', None) + if lookup_field is not None: + warnings.warn( + "The `Meta.lookup_field` option is pending deprecation. " + "Use `Meta.extra_kwargs={'url': {'lookup_field': ...}}` instead.", + PendingDeprecationWarning, + stacklevel=3 + ) + kwargs = extra_kwargs.get(field_name, {}) + kwargs['lookup_field'] = lookup_field + extra_kwargs[api_settings.URL_FIELD_NAME] = kwargs + + return extra_kwargs + def _get_default_field_names(self, declared_fields, model_info): return ( [model_info.pk.name] + diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index fd6da699f..6db371469 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -71,6 +71,17 @@ def get_field_kwargs(field_name, model_field): if model_field.help_text: kwargs['help_text'] = model_field.help_text + max_digits = getattr(model_field, 'max_digits', None) + if max_digits is not None: + kwargs['max_digits'] = max_digits + + decimal_places = getattr(model_field, 'decimal_places', None) + if decimal_places is not None: + kwargs['decimal_places'] = decimal_places + + if isinstance(model_field, models.TextField): + kwargs['style'] = {'type': 'textarea'} + if isinstance(model_field, models.AutoField) or not model_field.editable: # If this field is read-only, then return early. # Further keyword arguments are not valid. @@ -86,9 +97,6 @@ def get_field_kwargs(field_name, model_field): kwargs['choices'] = model_field.flatchoices return kwargs - if isinstance(model_field, models.TextField): - kwargs['style'] = {'type': 'textarea'} - if model_field.null and not isinstance(model_field, models.NullBooleanField): kwargs['allow_null'] = True @@ -171,14 +179,6 @@ def get_field_kwargs(field_name, model_field): validator = UniqueValidator(queryset=model_field.model._default_manager) validator_kwarg.append(validator) - max_digits = getattr(model_field, 'max_digits', None) - if max_digits is not None: - kwargs['max_digits'] = max_digits - - decimal_places = getattr(model_field, 'decimal_places', None) - if decimal_places is not None: - kwargs['decimal_places'] = decimal_places - if validator_kwarg: kwargs['validators'] = validator_kwarg diff --git a/rest_framework/utils/model_meta.py b/rest_framework/utils/model_meta.py index b6c41174e..7a95bcdd1 100644 --- a/rest_framework/utils/model_meta.py +++ b/rest_framework/utils/model_meta.py @@ -107,8 +107,8 @@ def get_field_info(model): related=relation.model, to_many=True, has_through_model=( - hasattr(relation.field.rel, 'through') and - not relation.field.rel.through._meta.auto_created + (getattr(relation.field.rel, 'through', None) is not None) + and not relation.field.rel.through._meta.auto_created ) )