Injection of read-only default values is done too late

[vdel]

refs #6053

The value returned by to_internal_value might not be a dictionary. In such cases, when passing the value from this line https://github.com/encode/django-rest-framework/blob/master/rest_framework/serializers.py#L434 to run_validators just below, it fails to call to_validate.update because value is not a dictionary.

The proposed fix is to inject the default values before converting the input data to internal value

[carltongibson]

Why the close?

[vdel]

Why the close?

I realized things were more intricated than what I initially thought and I'm not sure to have the time to dig this deeper. Here is my current understanding of the problem:

1. The to_internal_value here may return an object so we need to inject the default values for read-only before serializing to the internal value.
2. I guess injecting the read-only defaults directly in the data variable may not be suitable as the later may contain data formatted by an HTML form and we probably want to avoid mixing data in HTML format and more traditional dict/JSON format. Also, to_internal_value does not iterate over the read-only fields.
3. So I currently "hacked" things by adding a loop on the read-only fields to try to fill the data returned by to_internal_value with the read-only defaults as well. There is probably a nicer way to do so.
4. This breaks TestReadOnly.test_validate_read_only which checks that read-only fields are not included in validation. With my current solution, the read-only field is now included with its default value and to repair this tests we would need to put assert serializer.validated_data == {'read_only': '789', 'writable': 456} here. I do not think this would be suitable as this data has not been actually validated. (In theory it could be validated though, even if I do not quite see the point ?)

So I have the impression we are a bit stuck in a design issue here:

• on one side: the validators need the internal data format, which seems to imply we need to embed the default values of read-only fields as well, at least those with defaults?
• on the other side: we assume that validated_data does not embed read-only fields. I do not know if this a strong DRF assumption and if users of DRF rely on this.

How do you see things?

One solution could maybe be to rename to_internal_value into to_primitive_value, perform validation as done by #5922, and define to_internal_value as a wrapper of validated_data ?

[vdel]

One solution could maybe be to rename to_internal_value into to_primitive_value, perform validation as done by #5922, and define to_internal_value as a wrapper of validated_data ?

This works but would break the compatibility for everyone who overrides to_internal_data to return a custom python object as the validators are now applied to the dict of primitive values and not an object as before.

I won't have the time to dig deeper than that, hope this helps !

[rpkilby]

I won't have the time to dig deeper than that, hope this helps !

Thanks. I'm sure anyone digging into this will appreciate the legwork you've put into it.

[carltongibson]

Superseded by #6365. Thanks for the effort @vdel!