New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with nested schemas and custom validators #144

Closed
vovanbo opened this Issue Feb 9, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@vovanbo

vovanbo commented Feb 9, 2015

Let's assume we have following code:

from marshmallow import Schema, fields, ValidationError


class ThirdLevel(Schema):
    name = fields.String()


class SecondLevel(Schema):
    third = fields.Nested(ThirdLevel, many=True, required=True)

@SecondLevel.validator
def validate_emptiness_of_third(schema, input_data):
    if not input_data.get('third'):
        raise ValidationError('Custom error message from second level.', 'third')


class FirstLevel(Schema):
    second = fields.Nested(SecondLevel, many=True, required=True)

@FirstLevel.validator
def validate_emptiness_of_second(schema, input_data):
    if not input_data.get('second'):
        raise ValidationError('Custom error message from first level.', 'second')


good_data = {
    'second': [
        {
            'third': [
                {
                    'name': 'Some'
                }
            ]
        }
    ]
}


bad_data = {
    'second': [
        {
            'third': None
        }
    ]
}


result, errors = FirstLevel().load(good_data)

print(result)  # Output: {'second': [{'third': [{'name': u'Some'}]}]}

result, errors = FirstLevel().load(bad_data)  # Raises AttributeError

Last serialization raises AttributeError: 'dict' object has no attribute 'extend', because of fields.Unmarshaller have errors field which looks before error like:

{'second': {'third': [u'Field may not be null.', 'Custom error message from second level.']}}

So, it's really dict which can't be extended with list of err.messages here.

@sloria sloria added the bug label Feb 12, 2015

@sloria

This comment has been minimized.

Member

sloria commented Feb 12, 2015

Thanks for reporting this. I will look into this sometime this weekend.

@sloria

This comment has been minimized.

Member

sloria commented Feb 15, 2015

FWIW, in marshmallow 2.0-a (currently the dev branch), you can customize the error message for allow_none validation. The following gives the desired output:

from marshmallow import Schema, fields

class ThirdLevel(Schema):
    name = fields.String()

class SecondLevel(Schema):
    third = fields.Nested(ThirdLevel, 
                          many=True, 
                          required=True,
                          allow_none='Custom error message from second level')

class FirstLevel(Schema):
    second = fields.Nested(SecondLevel, 
                           many=True, 
                           required=True,
                           allow_none='Custom error message from the first level')

bad_data = {
    'second': [
        {
            'third': None
        }
    ]
}

result, errors = FirstLevel().load(bad_data)
assert errors == {'second': {'third': ['Custom error message from second level']}}

That said, the bug you reported with using schema-level validators still exists. Will look into this further.

sloria added a commit that referenced this issue Feb 15, 2015

@sloria

This comment has been minimized.

Member

sloria commented Feb 15, 2015

I have made a bugfix to handle nested schema validators. Using the above schemas, the output would be:

result, errors = FirstLevel().load(bad_data)
assert errors == {
    'second': {'third': ['Custom error message from second level.'], 
               '_schema': ['Custom error message from first level.']}
}

Since the nested field second is a dictionary rather than a list, we store schema-level errors on the _schema key. Note that the input_data passed to validate_emptiness_of_second is {'second': None}. Because validation failed for SecondLevel, the value for second becomes None.

The bug is fixed, but for the above use case, allow_none is more appropriate than schema-level validators.

@sloria sloria closed this Feb 15, 2015

@vovanbo

This comment has been minimized.

vovanbo commented Feb 15, 2015

I'm already using allow_none, but I don't know that string can be passed as message for validation error. Great! Thank you for good and useful tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment