Cannot re-validate validated_data in DRF

[phillipuniverse]

This is a bit of a specific issue with writable nested serializers and the changed behavior in #523. Since 6.4.0 many of my phone number fields now fail validation.

Here is a very contrived serializer makeup that illustrates the problem:

class NestedSerializer(Serializer):
    primary_phone_number = FixedPhoneNumberField(required=False, allow_null=True, allow_blank=True)

    def create(self, validated_data):
        # do lots of business logic
        return validated_data


class ParentSerializer(Serializer):
    nested = NestedSerializer(write_only=True)
    anotherfield = CharField()

    def create(self, validated_data):
        nested_instance = NestedSerializer(data=validated_data.pop("nested"))
        nested_instance.is_valid(raise_exception=True)
        nested_instance.save()
        return validated_data

Here's an example of using these serializers:

class PhoneNumberRegressionTestCase(TestCase):

    def test_serializers(self):
        parent = ParentSerializer(data={"nested": {"primary_phone_number": "+15126121234"}, "anotherfield": "some data"})
        parent.is_valid(raise_exception=True)
        parent.save()

When I run this code, I get back a ValidationError on nested.primary_phone_number:

  File "/Users/phillip/demo-proj/tests/test_serializers.py", line 319, in create
    nested_instance.is_valid(raise_exception=True)
  File "/Users/phillip/Library/Caches/pypoetry/virtualenvs/demo-proj-cpwchxt0-py3.10/lib/python3.10/site-packages/rest_framework/serializers.py", line 235, in is_valid
    raise ValidationError(self.errors)
rest_framework.exceptions.ValidationError: {'primary_phone_number': [ErrorDetail(string='Enter a valid phone number.', code='invalid')]}

Digging into this, this is because the DRF CharField only accepts a str, int, float or bool. In the change in #523, we unconditionally pass the data field to the super method. However, in this piece of code in the create():

    def create(self, validated_data):
        nested_instance = NestedSerializer(data=validated_data.pop("nested"))

The primary_phone_number entry is already a PhoneNumber because of the implementation of to_internal_value. Initializing the NestedSerializer again with the validated_data runs through validation again (with the is_valid call) and the new call to super().to_initial_value() fails.

I thought perhaps the fix would be to use self.initial_data instead of validated_data which has the original string instance, but this goes against the DRF docs of how to use writable nested serializers.

Here is a workaround that fixes the issue and is as flexible as versions < 6.4.0:

from phonenumber_field.phonenumber import to_python
from phonenumber_field.phonenumber import PhoneNumber
from rest_framework.exceptions import ValidationError

class FixedPhoneNumberField(PhoneNumberField):

    def to_internal_value(self, data):

        if isinstance(data, PhoneNumber):
            phone_number = data
        else:
            str_value = super().to_internal_value(data)
            phone_number = to_python(str_value, region=self.region)

        if phone_number and not phone_number.is_valid():
            raise ValidationError(self.error_messages["invalid"])
        return phone_number

[phillipuniverse]

One thing I realized as I typed this out - I could potentially just not call nested_instance.is_valid(raise_exception=True). But while this is a contrived example, in my real implementation I have a lot of logic within the NestedSerializer.create() method so I need that create() method to be executed. But if I try to nested_instance.save() without first calling is_valid(), I get this from DRF:

AssertionError: You must call `.is_valid()` before calling `.save()`.

[phillipuniverse]

I added a fix in #543. In the meantime, here is a monkeypatch that works, I have this in my settings.py:

###
### Start workaround for https://github.com/stefanfoulis/django-phonenumber-field/issues/542
###
import phonenumber_field.serializerfields as phonenumber_serializerfields
from phonenumber_field.phonenumber import PhoneNumber, to_python
from rest_framework.exceptions import ValidationError
from rest_framework.fields import CharField


def _relaxed_flexible_to_internal_value(self, data):
    if isinstance(data, PhoneNumber):
        phone_number = data
    else:
        str_value = super(phonenumber_serializerfields.PhoneNumberField, self).to_internal_value(
            data
        )
        phone_number = to_python(str_value, region=self.region)

    if phone_number and not phone_number.is_valid():
        raise ValidationError(self.error_messages["invalid"])
    return phone_number


setattr(
    phonenumber_serializerfields.PhoneNumberField,
    "to_internal_value",
    _relaxed_flexible_to_internal_value,
)

###
### End workaround
###

[francoisfreitag]

Thanks for the detailed report. I can reproduce the issue with the elements you’ve provided. Calling the validation twice looks incorrect to me, but I haven’t had time to dig into this issue today to understand whether it’s something from the serializers, from DRF or this library.
I’ll be away for a good week and back on Dec 5th, I’ll investigate then and report back.

[francoisfreitag]

Looks like DRF also handles getting a richer type as input to to_internal_value():

• https://github.com/encode/django-rest-framework/blob/cc3c89a11c7ee9cf7cfd732e0a329c318ace71b2/rest_framework/fields.py#L830-L841 (UUIDField)
• https://github.com/encode/django-rest-framework/blob/cc3c89a11c7ee9cf7cfd732e0a329c318ace71b2/rest_framework/fields.py#L1157-L1177 (DateTimeField)
• https://github.com/encode/django-rest-framework/blob/cc3c89a11c7ee9cf7cfd732e0a329c318ace71b2/rest_framework/fields.py#L1212-L1239 (DateField)
• many others...

Other fields also are written in a manner that would silently accept a richer (or pre-processed) type. The suggestion makes sense to me, please go ahead and add a test to the PR, I’ll happily include your work and release a bugfix. Thank you!

[phillipuniverse]

@francoisfreitag will do! Thanks so much for the thoughtful review!