django-rest-framework icon indicating copy to clipboard operation
django-rest-framework copied to clipboard

Partial updates and object-level validations

Open ecordell opened this issue 10 years ago • 6 comments

The current design of rest framework doesn't seem to support object-level validations on partial updates.

Quick example:

from rest_framework import serializers

class ItemInListSerializer(serializers.Serializer):
    options=serializers.ListField()
    item=serializers.CharField()

    def validate(self, attrs):
        options = attrs.get('options')
        item = attrs.get('item')

       if item not in options:
            raise serializers.ValidationError('Item is not a valid option!')

    def update(self, instance, validated_data):
        instance.attributes = validated_data
        return instance

    def create(self, validated_data):
        # create some object

This is just a toy example, but the issue should be pretty clear: if you're creating an object with the serializer, then attrs contains both options and item, and the validations pass and the object is created. If you attempt a partial update to set item (for example), the validate method will always run before update, and will always fail, because options does not exist.

Is there any standard solution to this scenario? I could certainly write validations before anything gets to the serializer, or push them down into models, but it seems like (and correct me if I'm wrong) DRF is attempting to be that validation layer and, in theory, these validations should and could live on the serializer.

Also just to note, I'm not the only person with this problem, I found this on SO: http://stackoverflow.com/questions/28646454/django-rest-framework-partial-update-and-validation

Any help would be appreciated! Thanks!

ecordell avatar Jun 24 '15 00:06 ecordell

You're correct that it's a little bit awkward if you want to write validation behavior that supports both full and partial updates. You can however do so by falling back getting the existing value from self.instance

Eg I think this looks right?...

def validate(self, attrs):
    options = attrs.get('options', self.instance.options)
    item = attrs.get('item', self.instance.item)

   if item not in options:
        raise serializers.ValidationError('Item is not a valid option!')

Hope that's a sufficient explanation. I'll label this as a documentation ticket for now.

TODO: review if there's an obvious and appropriate place that we could include a better example around this.

Related: I've reopened the ticket to allow a global disabling of PATCH support... #2979

tomchristie avatar Jun 24 '15 08:06 tomchristie

That code fails when creating because self.instance is None.

jjconti avatar May 20 '16 15:05 jjconti

@jjconti so simply:

def validate(self, attrs):
        subtype = attrs.get('subtype')
        promo_code = attrs.get('promo_code')
        date_start = attrs.get('date_start')
        date_end = attrs.get('date_end')

        if self.partial and self.instance:
            subtype = attrs.get('subtype', self.instance.subtype)
            promo_code = attrs.get('promo_code', self.instance.promo_code)
            date_start = attrs.get('date_start', self.instance.date_start)
            date_end = attrs.get('date_end', self.instance.date_end)

        # validate your data here

however thats so much repetitive code that deserves to be abstracted somehow

pySilver avatar Jul 17 '16 23:07 pySilver

Hi, i stumbled upon this issue while having it. The issue is kinda old now but i want to give my view on this topic.

In my opinion there are two different issues here :

  1. General problem with non-existent data while partially updating

  2. Unambiguous behaviour.

To 1.: As @pySilver posted there is a solution even if it is very redundant. I have 2 different solutions :

I. Skip validate() method in rest_framework/serialisers.py run_validation() method if self.partial is True.

Idea A:

    def run_validation(self, data=empty):
            """
            We override the default `run_validation`, because the validation
            performed by validators and the `.validate()` method should
            be coerced into an error dictionary with a 'non_fields_error' key.
              """
              (is_empty_value, data) = self.validate_empty_values(data)
              if is_empty_value:
                     return data

             value = self.to_internal_value(data)
             try:
                 self.run_validators(value)
                 if not self.partial:
                    # object-level validation not available during partial updates
                     value = self.validate(value)
                 assert value is not None, '.validate() should return the validated data'
            except (ValidationError, DjangoValidationError) as exc:
                raise ValidationError(detail=as_serializer_error(exc))

            return value

Idea B: Introducing a validate_partial(self, attrs) which is called during self.partial=True validations. In this case each User has the opportunity to insert validation logic for partial-updates.

II. Regarding ambiguity :

Thinking about partial updates the validate() method, since it's object-level validation, becomes obsolete. On a partial Update a Developer can not really be sure that the "to-be-checked" attributes are always given. To check consistency with the current values one would need a switch to self.instance.<attr-of-interest> as @pySilver suggested. We could solve that by a method get_from_instance(field_name) which if self.instance is not None retrieves the current field value otherwise returns None.

With this method @pySilver 's code would be generalized to :

def validate(self, attrs):
    subtype = attrs.get('subtype', self.get_from_instance("subtype"))
    promo_code = attrs.get('promo_code', self.get_from_instance("promo_code"))
    date_start = attrs.get('date_start', self.get_from_instance("date_start"))
    date_end = attrs.get('date_end', self.get_from_instance("date_end"))

This should then become a guideline for "How to customize the validate() method?" since it would work for creation, updating (full) and partial updates.

Please, if anyone reads this, let me know what you think about this. I'd be happy to contribute a PR in response.

chgad avatar Feb 17 '19 18:02 chgad

A workaround for anyone else who found themselves on this thread: I defined a utility method stupid_get(attr_name) in the one place I needed it.

    def validate(self, data):

        def stupid_get(attr_name):
            if attr_name in data:
                return data[attr_name]
            if self.instance and hasattr(self.instance, attr_name):
                return getattr(self.instance, attr_name)
            return None

        my_local_var = stupid_get('var_name')

This could be adapted to be standalone by changing the signature to stupid_get(instance, data, attr_name) if you need to use it in many places.

pvillano avatar Jul 26 '19 14:07 pvillano

Hello, I have came across this problem as well, and this thread was life saving! It would be great if the docs could be updated. I have a little bit different approach (sticking to the official example):

def validate(self, data):
    """
    Check that start is before finish.
    """
    # only if 'start' cannot contain values that can be evaluated to false
    start = data.get('start') or self.instance.start

    # if falsy values are possible
    finish = data['finish'] if data.get('finish') is not None else self.instance.finish

    if start > finish:
        raise serializers.ValidationError("finish must occur after start")
    return data

It is not as safe as the above stupid_get but shorter and works for certain scenarios.

hodossy avatar Feb 26 '20 08:02 hodossy