Improve introspection of reverse relationships for serializer generation.

[GeeWee]

Checklist

• I have verified that that issue exists against the master branch of Django REST framework. (tested against 3.9.0 from pip)
• I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• This is not a usage question. (Those should be directed to the discussion group instead.)
• This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• I have reduced the issue to the simplest possible case.
• I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.) ( Not sure how the tests are structured in DRF, but I have included a failing test in code)

Steps to reproduce

Note that this is a potential duplicate/regression of #2017

DRF's serializers do not seem to respect null/blank in relation to the reverse side of the OneToOneField. I'm unsure whether this is with this field in particular, or a more general thing.

Take the following models

# models.py
class Location(models.Model):
    pass

class Company(models.Model):
    main_location = models.OneToOneField(to=Location, null=True, blank=True, on_delete=models.SET_NULL, related_name="owning_company")

And the following serializer for Locations

# Serializers.py
class LocationSerializer(serializers.ModelSerializer):
    class Meta:
        model = Location
        fields = ("id", "owning_company")

I would expect that DRF correctly realizes that the inverse part of the relationship is not required. I would expect the following test to pass

class TestLocationEndpoint(APITestCase):
    def test_location_can_be_created(self):
        # Should be able to instantiate the object without any data at all.
        serializer = LocationSerializer(data = {})
        serializer.is_valid(raise_exception=True)

However it fails with the following validationError:
rest_framework.exceptions.ValidationError: {'owning_company': [ErrorDetail(string='Dette felt er påkrævet.', code='required')]}
(Don't mind the translation)

If I manually set required=false on the owning_company field, there is no problem:
This serializer works fine

class LocationSerializer(serializers.ModelSerializer):
    owning_company = serializers.PrimaryKeyRelatedField(required=False, queryset=Company.objects.all())

    class Meta:
        model = Location
        fields = ("id", "owning_company")

Expected behavior

I expect DRF to pick up on the fact that the field is set to blank in the model, and therefore required should not be set to true as default.

Actual behavior

Required is set to true, no matter what the value of blank and null is in the model.

[rpkilby]

Thanks for the report. I've assigned this to myself to keep track of the issue, but don't know when I'll be able to take a look at this. If anyone wants to take a stab at this, please feel free to.

[rpkilby]

Also assigning to the milestone to make sure this gets reviewed.

[anomitra]

As per the docs required defaults to True. In light of that, do we still want to let a ModelSerializer pick up settings from it's model?

@rpkilby @GeeWee

[tomchristie]

The behaviour ought to be that required is set if the model field does not have a default value. Requiredness is slightly different to the semantics of which values are allowed. Blank=True means that the empty string is a valid value. Null=True means that None is a valid value.

[GeeWee]

It's interesting because the comparison between required and blank kinda breaks down. My impression is that in e.g. a ModelForm, an empty field is basically just an empty string "" - which, depending on the Field, means different things. E.g. a CharField considers it None internally, whereas GenericIpAddressField just saves it as null in the database.

So while technically blank=true should just mean "You can submit an empty string here and it's a-okay" - I'd consider that a relic of the forms implementation which sends the empty string along. I think a more pragmatic approach is "I don't care if you send me this value or not"

However, I am reasonably new (<6 months) to both Django and DRF, so this might all be horribly wrong.

[anomitra]

required is set if the model field does not have a default value

Going by this semantic, I think the issue can be called expected behaviour?

[carltongibson]

AFAICS, this is the expected behaviour for required. Nonetheless, the underlying introspection could probably be enhanced a little...

You want to be sending something like this:

serializer = LocationSerializer(data = {
   'owning_company': None,
})

i.e. the field is required (because there's no default) but None is an accepted value.

This doesn't work however:

>>> from issue.serializers import LocationSerializer
>>> data = {'owning_company':None}
>>> l = LocationSerializer(data=data)
>>> l
LocationSerializer(data={'owning_company': None}):
    id = IntegerField(label='ID', read_only=True)
    owning_company = PrimaryKeyRelatedField(queryset=Company.objects.all())
>>> l.is_valid()
False
>>> l.errors
{'owning_company': [ErrorDetail(string='This field may not be null.', code='null')]}

The issue is that RelationInfo.model_field is None for reverse relationships, so the correct kwargs are not generated.

If someone wants to have a pop at checking reverse and using to_field in this case, that would be interesting to look at.

Given that this behaviour is unchanged since (at least) before v3.0 it's not something I want to consider a priority.