serializers as fields not used properly?

[syphar]

I discovered this when working with ListOrItemField and wanted to know if this usage-pattern is excluded by design or if this is a bug/feature. (and what solution is preferred before I start working on a PR).

Example for my problem:

class SubSerializer(serializers.Serializer):
    field_1 = serializers.IntegerField()

class MainSerializer(serializers.Serializer):
    testfield = ListOrItemField(item_field=SubSerializer())

• The data is parsed and converted correctly
• but the validation logic is not applied

Is this field-type only designed to work with other fields? Or should we fix this?

[estebistec]

I would expect that to work. I don't really want to exclude any features on sub-serializers. Feel free to start a PR and I'll support where I can.

[estebistec]

@syphar Do you have a particular test that reproduces this? I could poke at it myself too if you aren't too far along.

[syphar]

@estebistec I wrote a small test-case here.

https://github.com/fasterweb/drf-compound-fields/blob/substructure/tests/test_in_serializers.py#L91-122

if everything is ok, the data is parsed perfect (first testcase). But the second test-case fails since is_valid is True for invalid data.

[kevin-brown]

Serializers rely on field_from_native and field_to_native being called, which is where validation typically occurs. drf-compound-fields only calls from_native and to_native which doesn't do the validation.

It is mentioned in the docstring that serializers should just use the many option, so this may be why.

[syphar]

My usecase was ListOrItemField, which can't be replaced with many=True

Sent from Mobile

[estebistec]

Taking a look to see what I can do here...

[estebistec]

So, giving field_from_native a shot when the embedded field is a serializer. I get a type error on this line:

https://github.com/estebistec/django-rest-framework/blob/master/rest_framework/serializers.py#L442

because self.parent is None on the embedded serializer. @kevin-brown, the code there looks nearly like it's okay with not having a parent, but instead assumes that it's there to check for an object attribute. Is that a bug in BaseSerializer or should I always make sure to provide a parent?

[kevin-brown]

My usecase was ListOrItemField

My bad, I was assuming that the same was mentioned in the ListOrItemField docstring, the opposite is true though.

because self.parent is None on the embedded serializer

field.parent is set using field.initialize(), which should be called before field.field_from_native().

field.initialize call: https://github.com/tomchristie/django-rest-framework/blob/91eabd54bbc42e8a2540db2ff070097db7a0f4a0/rest_framework/serializers.py#L281 definition: https://github.com/tomchristie/django-rest-framework/blob/91eabd54bbc42e8a2540db2ff070097db7a0f4a0/rest_framework/fields.py#L167-L179

[estebistec]

Okay, cool. I'll play with that some more (probably tomorrow). Thanks!

[estebistec]

Okay, sorry, took an extra day to get to it.

check out the proposed fixed over in #16.

@syphar Do you have more test cases you'd like to see supported?

@kevin-brown thanks for the tips. I implemented intialize and field_from_native to simply switch between an item or list field as appropriate. I think I can get away without implementing field_to_native or run_validators. Does that sound right?

[kevin-brown]

I think I can get away without implementing field_to_native or run_validators

You can get away without overriding field_to_native, that doesn't do anything special. I'm not sure about run_validators though, as you may want to split that away from the validate logic.

[estebistec]

@kevin-brown Yeah, that makes sense. Updated the pull request. And to maybe go one final step, is there any reason I shouldn't similar split out run_validators for ListField and DictField?

[estebistec]

16 fixes this. Thanks again to @kevin-brown for the advice on the fixes.

[estebistec]

Fixed released in 0.2.2: https://pypi.python.org/pypi/drf-compound-fields/0.2.2