TypeError with partial=True

[kkirss]

MR #18 created a new issue.

Consider this simple serializer:

class MyPolymorphicSerializer(PolymorphicSerializer):
    pass  # Mostly irrelevant

class MySerializer(Serializer):
    foo = CharField()
    bar = MyPolymorphicSerializer(required=false)

I use standard DRF viewsets to do a PATCH request (i.e. serializer has partial=True) to an endpoint using MySerializer, with JSON content: {"foo": "my_foo"}. We exclude bar, which for PATCH requests should mean that this field is virtually ignored and kept as it is before the request. This worked as expected before this MR.

Now however, it will raise this error:

  File "/usr/local/lib/python3.8/site-packages/rest_framework/mixins.py", line 67, in update
    serializer.is_valid(raise_exception=True)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 234, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 433, in run_validation
    value = self.to_internal_value(data)
  File "/usr/local/lib/python3.8/site-packages/rest_framework/serializers.py", line 490, in to_internal_value
    validated_value = field.run_validation(primitive_value)
  File "/usr/local/lib/python3.8/site-packages/rest_polymorphic/serializers.py", line 95, in run_validation
    resource_type = self._get_resource_type_from_mapping(data)
  File "/usr/local/lib/python3.8/site-packages/rest_polymorphic/serializers.py", line 112, in _get_resource_type_from_mapping
    return mapping[self.resource_type_field_name]
TypeError: 'type' object is not subscriptable

This happens because normally, Serializer.run_validation does checks (validate_empty_values) to ensure there is some data to begin with. Now however, it tries to determine the resource_type before running the base implementation. When determining the resource_type, it tries to do empty['somekey'], which of course results in a TypeError.

Note that there might be other similar edge-cases I didn't cover here.

IMO this issue is more critical than the one solved by #18 in the first place.

[denisorehovsky]

Hi. Thanks. I haven't used this library for a long time and I'm not interested in doing any fixes right now. If you have some free time, you can send a PR if you want. I will take a look. Thanks

[iglosiggio]

I'm not working w/DRF nor django-polymorphic rigth now. Is there any reason why adding a check for empty data on my previous PR wouldn't fix your problem?

I'll try to come up with a fix later :)

[denisorehovsky]

@iglosiggio @runekri3 Thank you, guys.

I will appreciate if there will be some tests that cover this issue.

[kkirss]

I think adding that check should fix the problem indeed. I'd write more comprehensive tests to make sure tho. Can also check the PR if you create one :)

[askoretskiy]

I have the same problem. When running the validation on partial=True and type is not given -- it crashes.

I could reproduce the problem on django-rest-polymorphic==0.1.9 and also on master branch.

Basically sometimes data is not provided to run_validation. Then empty is used:

 94  	    def run_validation(self, data=empty):
 95  ->	        resource_type = self._get_resource_type_from_mapping(data)
 96  	        serializer = self._get_serializer_from_resource_type(resource_type)
 97  	        validated_data = serializer.run_validation(data)
 98  	        validated_data[self.resource_type_field_name] = resource_type
 99  	        return validated_data

When trying to extract type from empty -- it gives TypeError:

110  	    def _get_resource_type_from_mapping(self, mapping):
111  	        try:
112  >>	            return mapping[self.resource_type_field_name]
113  	        except KeyError:
114  ->	            raise serializers.ValidationError({
115  	                self.resource_type_field_name: 'This field is required',
116  	            })

[iglosiggio]

I think https://github.com/apirobot/django-rest-polymorphic/pull/31 should fix this issue. It seems that we need to add a good test for keeping the coverage high but other than that the fix looks good to me.

[OllyPage]

as this is the only reference to this on google here is a bodge to fix the issue with 0.1.9

In a serializer:

def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
       
        if self.context['request'].method == "PATCH":
            if "dossier" not in kwargs.get("data", {}).keys():
                self.fields['polymorphic_field'].read_only = True