From c6137bbf5aa7ca800e4afc06657e5196b2e0e481 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 17 Dec 2014 14:14:51 +0000 Subject: [PATCH] Serializer API restrictions. --- docs/api-guide/serializers.md | 6 +++++ rest_framework/generics.py | 8 +++--- rest_framework/renderers.py | 16 ++++++----- rest_framework/serializers.py | 51 +++++++++++++++++++++++++++++------ tests/test_bound_fields.py | 2 +- 5 files changed, 62 insertions(+), 21 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index 5fe6b4c29..137cc9d5f 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -240,6 +240,12 @@ Serializer classes can also include reusable validators that are applied to the For more information see the [validators documentation](validators.md). +## Accessing the initial data and instance + +When passing an initial object or queryset to a serializer instance, the object will be made available as `.instance`. If no initial object is passed then the `.instance` attribute will be `None`. + +When passing data to a serializer instance, the unmodified data will be made available as `.initial_data`. If the data keyword argument is not passed then the `.initial_data` attribute will not exist. + ## Partial updates By default, serializers must be passed values for all required fields or they will raise validation errors. You can use the `partial` argument in order to allow partial updates. diff --git a/rest_framework/generics.py b/rest_framework/generics.py index 3d6cf1684..e6db155e7 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -79,16 +79,14 @@ class GenericAPIView(views.APIView): 'view': self } - def get_serializer(self, instance=None, data=None, many=False, partial=False): + def get_serializer(self, *args, **kwargs): """ Return the serializer instance that should be used for validating and deserializing input, and for serializing output. """ serializer_class = self.get_serializer_class() - context = self.get_serializer_context() - return serializer_class( - instance, data=data, many=many, partial=partial, context=context - ) + kwargs['context'] = self.get_serializer_context() + return serializer_class(*args, **kwargs) def get_pagination_serializer(self, page): """ diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index cfcf1f5d0..634338e9e 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -544,12 +544,12 @@ class BrowsableAPIRenderer(BaseRenderer): # serializer instance, rather than dynamically creating a new one. if request.method == method and serializer is not None: try: - data = request.data + kwargs = {'data': request.data} except ParseError: - data = None + kwargs = {} existing_serializer = serializer else: - data = None + kwargs = {} existing_serializer = None with override_method(view, request, method) as request: @@ -569,11 +569,13 @@ class BrowsableAPIRenderer(BaseRenderer): serializer = existing_serializer else: if method in ('PUT', 'PATCH'): - serializer = view.get_serializer(instance=instance, data=data) + serializer = view.get_serializer(instance=instance, **kwargs) else: - serializer = view.get_serializer(data=data) - if data is not None: - serializer.is_valid() + serializer = view.get_serializer(**kwargs) + + if hasattr(serializer, 'initial_data'): + serializer.is_valid() + form_renderer = self.form_renderer_class() return form_renderer.render( serializer.data, diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index e9860a2fc..8de22f4b9 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -58,11 +58,31 @@ class BaseSerializer(Field): """ The BaseSerializer class provides a minimal class which may be used for writing custom serializer implementations. + + Note that we strongly restrict the ordering of operations/properties + that may be used on the serializer in order to enforce correct usage. + + In particular, if a `data=` argument is passed then: + + .is_valid() - Available. + .initial_data - Available. + .validated_data - Only available after calling `is_valid()` + .errors - Only available after calling `is_valid()` + .data - Only available after calling `is_valid()` + + If a `data=` argument is not passed then: + + .is_valid() - Not available. + .initial_data - Not available. + .validated_data - Not available. + .errors - Not available. + .data - Available. """ - def __init__(self, instance=None, data=None, **kwargs): + def __init__(self, instance=None, data=empty, **kwargs): self.instance = instance - self._initial_data = data + if data is not empty: + self.initial_data = data self.partial = kwargs.pop('partial', False) self._context = kwargs.pop('context', {}) kwargs.pop('many', None) @@ -156,9 +176,14 @@ class BaseSerializer(Field): (self.__class__.__module__, self.__class__.__name__) ) + assert hasattr(self, 'initial_data'), ( + 'Cannot call `.is_valid()` as no `data=` keyword argument was' + 'passed when instantiating the serializer instance.' + ) + if not hasattr(self, '_validated_data'): try: - self._validated_data = self.run_validation(self._initial_data) + self._validated_data = self.run_validation(self.initial_data) except ValidationError as exc: self._validated_data = {} self._errors = exc.detail @@ -172,6 +197,16 @@ class BaseSerializer(Field): @property def data(self): + if hasattr(self, 'initial_data') and not hasattr(self, '_validated_data'): + msg = ( + 'When a serializer is passed a `data` keyword argument you ' + 'must call `.is_valid()` before attempting to access the ' + 'serialized `.data` representation.\n' + 'You should either call `.is_valid()` first, ' + 'or access `.initial_data` instead.' + ) + raise AssertionError(msg) + if not hasattr(self, '_data'): if self.instance is not None and not getattr(self, '_errors', None): self._data = self.to_representation(self.instance) @@ -295,11 +330,11 @@ class Serializer(BaseSerializer): return getattr(getattr(self, 'Meta', None), 'validators', []) def get_initial(self): - if self._initial_data is not None: + if hasattr(self, 'initial_data'): return OrderedDict([ - (field_name, field.get_value(self._initial_data)) + (field_name, field.get_value(self.initial_data)) for field_name, field in self.fields.items() - if field.get_value(self._initial_data) is not empty + if field.get_value(self.initial_data) is not empty and not field.read_only ]) @@ -447,8 +482,8 @@ class ListSerializer(BaseSerializer): self.child.bind(field_name='', parent=self) def get_initial(self): - if self._initial_data is not None: - return self.to_representation(self._initial_data) + if hasattr(self, 'initial_data'): + return self.to_representation(self.initial_data) return [] def get_value(self, dictionary): diff --git a/tests/test_bound_fields.py b/tests/test_bound_fields.py index 469437e4b..bfc54b233 100644 --- a/tests/test_bound_fields.py +++ b/tests/test_bound_fields.py @@ -22,7 +22,7 @@ class TestSimpleBoundField: amount = serializers.IntegerField() serializer = ExampleSerializer(data={'text': 'abc', 'amount': 123}) - + assert serializer.is_valid() assert serializer['text'].value == 'abc' assert serializer['text'].errors is None assert serializer['text'].name == 'text'