Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unusual behavior with is_valid #7

Open
Viicos opened this issue Feb 9, 2024 · 0 comments
Open

Unusual behavior with is_valid #7

Viicos opened this issue Feb 9, 2024 · 0 comments

Comments

@Viicos
Copy link

Viicos commented Feb 9, 2024

With the following example:

class V1Serializer(serializers.Serializer):
    v1_required_field = serializers.CharField()

class V2Serializer(serializers.Serializer):
    v2_optional_field = serializers.CharField(required=False)

class MainSerializer(PolymorphicSerializer):
    discriminator_field = "version"
    serializer_mapping = {
        1: V1Serializer,
        2: V2Serializer,
    }

    version = serializers.ChoiceField(
        choices=[1, 2],
        default=1,
    )
    main_field = serializers.CharField(required=False)     

I get some unusual behaviors when doing:

MainSerializer(data={"version": "v1"}).is_valid()
#> KeyError: "`MainSerializer.serializer_mapping` is missing a corresponding serializer for the `<class 'rest_framework.fields.empty'>` key"

The is_valid override:

def is_valid(self, *args, **kwargs):
valid = super().is_valid(*args, **kwargs)
extra_serializer = self._get_serializer_from_data(self.validated_data)
if extra_serializer is None:
return valid
extra_valid = extra_serializer.is_valid(*args, **kwargs)
self._errors.update(extra_serializer.errors)
return valid and extra_valid

adds an extra layer of validation, by calling is_valid again on the mapped serializer from the discriminator. However, if the first super().is_valid() call failed, self.validated_data is set to {} by DRF, so drf-polymorphic is unable to retrieve the serializer from the mapping, which leads to the KeyError.

The question now is: why is super().is_valid() failing? In the example, the data provided to MainSerializer seems to comply, at first sight there is no reason for super().is_valid() to fail. The reason is that the DRF implementation of is_valid calls to_internal_value which is overridden in our case:

def to_internal_value(self, data):
default = super().to_internal_value(data)
serializer = self._get_serializer_from_data(data)
if serializer is None:
return default
extra = serializer.to_internal_value(data)
return {**default, **extra}

and calls to_internal_value on the mapped serializer, which will run validation on each field of this serializer.


So it's a bit of a mess, validation is run twice but simply removing the extra logic in is_valid is not enough as we still need to run the mapped serializer validators (as to_internal_value only runs fields validators).

However, I think better exception handling could be applied so that is_valid(raise_exception=False) doesn't raise unexpected errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant