Inconsistent value type when validating nested schema fields

[torotil]

Problem:

When a Nested schema has a validation error its @post_load hooks are not invoked bypassing any transformations done there. This means the parent schema’s @validates and @validate_schema hooks have to deal with both the transformed and untransformed child values.

Originally I encountered this using marshmallow-sqlalchemy with load_instance=True.

Steps to reproduce:

The code below shows the problem.

import dataclasses
import uuid

import marshmallow as ma
import pytest


@dataclasses.dataclass
class ChildModel:
    uuid: uuid.UUID
    name: str


class Child(ma.Schema):
    uuid = ma.fields.UUID(required=True)
    name = ma.fields.String(required=True)

    @ma.post_load
    def transform(self, data, **_kwargs):
        return ChildModel(**data)


class Parent(ma.Schema):
    child = ma.fields.Nested("Child")

    @ma.validates("child")
    def validate_child(self, value, **_kwargs):
        if value:
            # This fails when the child has a validation error as then its @post_load is not called.
            assert isinstance(value, ChildModel)


def test_load_valid_uuid():
    data = {"child": {"uuid": "c81505b4-258b-4912-b8bc-8ac913d56736", "name": "test"}}
    result = Parent().load(data)
    assert isinstance(result["child"], ChildModel)


def test_load_invalid_uuid():
    data = {"child": {"uuid": "invalid-uuid", "name": "test"}}
    with pytest.raises(ma.exceptions.ValidationError):
        result = Parent().load(data)

When the child schema validates then Parent.validate_child is called with a ChildModel instance, but if the child data is invalid it gets called with a dict instance instead.

[sloria]

this is expected behavior though perhaps undesirable or at least documentable. we explicitly test that @validates decorators run even when a nested child has errors here:

marshmallow/tests/test_schema.py

Lines 1976 to 1992 in 638bed5

	def test_all_errors_on_many_nested_field_with_validates_decorator(self):
	class Inner(Schema):
	req = fields.Raw(required=True)
	class Outer(Schema):
	inner = fields.Nested(Inner, many=True)
	@validates("inner")
	def validates_inner(self, data, **kwargs):
	raise ValidationError("not a chance")
	outer = Outer()
	with pytest.raises(ValidationError) as excinfo:
	outer.load({"inner": [{}]})
	errors = excinfo.value.messages
	assert "inner" in errors
	assert "_schema" in errors["inner"]

that said, i see that this is surprising behavior. i wonder if we should support @validates(skip_on_field_errors=True) like we have with @validates_schema 🤔 @torotil would that meet your use case?

[torotil]

Thanks for your quick response @sloria !

I think it makes totally sense to call the @validates callbacks in the outer schema in most cases, as this way you get as many validation errors as possible at once. In our case it’s easily possible to also add support for plain dicts so I will do just that.

Adding the flag definitely sounds like a very good idea, especially since it’s even True by default for @validates_schema.

[sloria]

@torotil would your use case make use of skip_on_field_errors, or do you need the partially-valid dict data to be passed to the validator?

[torotil]

I chose to embrace also validating the dict because it’s nice to have. I wouldn’t have looked for it though, and the current behaviour was surprising.

The error about the unexpected input data almost made it to our live setup and would have caused a bug there. That’s why I still think it would be a very good idea to change the API, even if my use case doesn’t demand it.