List validation failing for strings

[mwarkentin]

We ran across a small bug with the ListField when a string is passed in:

def from_native():
    ...
    if self.item_field and data:
        return [self.item_field.from_native(item_data) for item_data in data]

This was causing a List of characters from the string to be passed into validate, so if not isinstance(data, list): wasn't actually being triggered. We moved the list check up into from_native, and it seems to be working well.

[estebistec]

Just to be clear, are you calling is_valid prior to accessing deserialized data, as prescribed by [1]. Are you testing the ListField on a model, or in isolation by calling from_native? If you can post the simplest, distilled version of your code reproducing the error, that would be helpful too. Thanks!

[1] http://www.django-rest-framework.org/api-guide/serializers#validation

[mwarkentin]

@estebistec We are calling is_valid(). I'll try to put together a simple test case for you.

[mwarkentin]

Here's a small shell_plus session showing the error. recipients is a ListField made up of EmailFields. This is with code taken from your old pull request into django-rest-framework, will look at grabbing the latest from this repo and making sure it's still an issue.

In [1]: from facade.resources.invoice.serializers import InvoiceEmailSerializer

In [2]: data = {"recipients": "foo"}

In [3]: serializer = InvoiceEmailSerializer(data=data)

In [4]: serializer.is_valid()
Out[4]: False

In [5]: serializer.errors
Out[5]:
{'recipients': [
  u"[u'Enter a valid email address.']", # f
  u"[u'Enter a valid email address.']", # o
  u"[u'Enter a valid email address.']", # o
]"]}

[mwarkentin]

Just verified that this is still an issue with the latest from this repo.

[mwarkentin]

Here's what it looks like if I change it to be a list of CharFields instead of EmailFields:

In [1]: from facade.resources.invoice.serializers import InvoiceEmailSerializer

In [2]: data = {"recipients": "recipients@example.com"}

In [3]: serializer = InvoiceEmailSerializer(data=data)

In [4]: serializer.is_valid()
Out[4]: True

[estebistec]

Okay, So recipients is either a ListField(CharField) or a ListField(EmailField)? I would expect both to get validation errors, as ListField isn't meant to support list-or-value logic.

Either way, there is some unexpected behavior here that I'm looking into.

[estebistec]

BTW, if you do want list-or-item functionality, I originally had just such a field when I first submitted the pull to Django REST framework. I've logged #5 to add it back here. It will build upon the ListField.

[mwarkentin]

I've opened a pull request with an integration test for this (creates a full serializer and calls is_valid(): https://github.com/estebistec/drf-compound-fields/pull/6

[mwarkentin]

@estebistec I pulled that example from our code. recipients is always a list of EmailFields, I just changed it in order to show the validation wasn't working as expected.

[estebistec]

The related fix has been merged, so that closes this issue.

[mwarkentin]

[mwarkentin]

Anything else you're looking at getting in before the initial release?

[estebistec]

Welp, I was going to do the ListOrItem field and some documentation improvements.

However, if you'd like to consume it as is, I'd be willing to cut 0.1.0 tonight and move those things to a 0.2.0 milestone.

[estebistec]

I'd still likely want to go the rest of my list done by end of this weekend (now that I'm over the hump with the nested validation), so there'd be another release then.

What do you think?

[mwarkentin]

The stuff we'd like to use is available in the package now, so a release now would be great. At the same time, a few more days won't hurt us, so if you'd like to get a few more things in, that's fine too. :)

[mwarkentin]

And again, thanks for doing this!

[estebistec]

if you could use a release now, I'll just go ahead and do it. 0.1.0 tonight, and 0.2.0 with the rest later this weekend.

[estebistec]

Tagged: https://github.com/estebistec/drf-compound-fields/releases/tag/v0.1.0

Stand by for PyPI release...

[estebistec]

Released! https://pypi.python.org/pypi/drf-compound-fields/0.1.0

[mwarkentin]

[estebistec]

hehe :P