From 270155331029787c1ace3efc3fd123e88397cb57 Mon Sep 17 00:00:00 2001 From: Michael Mulqueen Date: Tue, 3 May 2022 18:06:43 +0100 Subject: [PATCH] Warn when using nested instance and many together, raise helpful error on deserialisations. https://github.com/marshmallow-code/marshmallow/issues/1982 --- src/marshmallow/exceptions.py | 8 ++++++-- src/marshmallow/fields.py | 13 +++++++++++- src/marshmallow/warnings.py | 4 ++++ tests/test_deserialization.py | 4 ++-- tests/test_fields.py | 17 +++++++++++++++- tests/test_schema.py | 37 +++++++++++++++++++++++++++++++---- 6 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/marshmallow/exceptions.py b/src/marshmallow/exceptions.py index 16611345a..040f81631 100644 --- a/src/marshmallow/exceptions.py +++ b/src/marshmallow/exceptions.py @@ -54,9 +54,13 @@ class registry. """ -class StringNotCollectionError(MarshmallowError, TypeError): +class FieldConfigurationError(MarshmallowError): + """Raised when trying to configure a field with bad options.""" + + +class StringNotCollectionError(FieldConfigurationError, TypeError): """Raised when a string is passed when a list of strings is expected.""" -class FieldInstanceResolutionError(MarshmallowError, TypeError): +class FieldInstanceResolutionError(FieldConfigurationError, TypeError): """Raised when schema to instantiate is neither a Schema class nor an instance.""" diff --git a/src/marshmallow/fields.py b/src/marshmallow/fields.py index 09df0d993..e6f96aa06 100644 --- a/src/marshmallow/fields.py +++ b/src/marshmallow/fields.py @@ -25,9 +25,10 @@ ValidationError, StringNotCollectionError, FieldInstanceResolutionError, + FieldConfigurationError, ) from marshmallow.validate import And, Length -from marshmallow.warnings import RemovedInMarshmallow4Warning +from marshmallow.warnings import RemovedInMarshmallow4Warning, FieldConfigurationWarning __all__ = [ "Field", @@ -551,6 +552,11 @@ def __init__( "Use `Nested(lambda: MySchema(...))` instead.", RemovedInMarshmallow4Warning, ) + if isinstance(nested, SchemaABC) and many: + warnings.warn( + 'Passing "many" is ignored if using an instance of a schema. Consider fields.List(fields.Nested(...)) instead.', + FieldConfigurationWarning, + ) self.nested = nested self.only = only self.exclude = exclude @@ -634,9 +640,14 @@ def _serialize(self, nested_obj, attr, obj, **kwargs): return schema.dump(nested_obj, many=many) def _test_collection(self, value): + if self.many and not self.schema.many: + raise FieldConfigurationError( + '"many" may not be used in conjunction with an instance in a nested field.' + ) many = self.schema.many or self.many if many and not utils.is_collection(value): raise self.make_error("type", input=value, type=value.__class__.__name__) + # Check whether many on the nested field and the schema are in conflict. def _load(self, value, data, partial=None): try: diff --git a/src/marshmallow/warnings.py b/src/marshmallow/warnings.py index 0da3c50d4..69306ede3 100644 --- a/src/marshmallow/warnings.py +++ b/src/marshmallow/warnings.py @@ -1,2 +1,6 @@ class RemovedInMarshmallow4Warning(DeprecationWarning): pass + + +class FieldConfigurationWarning(Warning): + pass diff --git a/tests/test_deserialization.py b/tests/test_deserialization.py index b5f87c183..1494608d1 100644 --- a/tests/test_deserialization.py +++ b/tests/test_deserialization.py @@ -1354,7 +1354,7 @@ class PetSchema(Schema): name = fields.Str() class StoreSchema(Schema): - pets = fields.Nested(PetSchema(), allow_none=False, many=True) + pets = fields.Nested(PetSchema, allow_none=False, many=True) sch = StoreSchema() errors = sch.validate({"pets": None}) @@ -1378,7 +1378,7 @@ class PetSchema(Schema): name = fields.Str() class StoreSchema(Schema): - pets = fields.Nested(PetSchema(), required=True, many=True) + pets = fields.Nested(PetSchema, required=True, many=True) sch = StoreSchema() errors = sch.validate({}) diff --git a/tests/test_fields.py b/tests/test_fields.py index 79fd01727..f8314671b 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -9,7 +9,8 @@ RAISE, missing, ) -from marshmallow.exceptions import StringNotCollectionError +from marshmallow.exceptions import StringNotCollectionError, FieldConfigurationError +from marshmallow.warnings import FieldConfigurationWarning from tests.base import ALL_FIELDS @@ -395,6 +396,20 @@ class MySchema(Schema): "nested": {"foo": "baz"} } + def test_load_instanced_nested_schema_with_many(self): + class NestedSchema(Schema): + foo = fields.String() + bar = fields.String() + + with pytest.warns(FieldConfigurationWarning): + + class MySchema(Schema): + nested = fields.Nested(NestedSchema(), many=True) + + schema = MySchema() + with pytest.raises(FieldConfigurationError): + schema.load({"nested": [{"foo": "123", "bar": "456"}]}) + class TestListNested: @pytest.mark.parametrize("param", ("only", "exclude", "dump_only", "load_only")) diff --git a/tests/test_schema.py b/tests/test_schema.py index c00189df2..532d822fa 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -24,6 +24,7 @@ StringNotCollectionError, RegistryError, ) +from marshmallow.warnings import FieldConfigurationWarning from tests.base import ( UserSchema, @@ -2165,11 +2166,23 @@ class ParentSchema(Schema): class TestPluckSchema: - @pytest.mark.parametrize("user_schema", [UserSchema, UserSchema()]) - def test_pluck(self, user_schema, blog): + def test_dump_cls_pluck(self, blog): class FlatBlogSchema(Schema): - user = fields.Pluck(user_schema, "name") - collaborators = fields.Pluck(user_schema, "name", many=True) + user = fields.Pluck(UserSchema, "name") + collaborators = fields.Pluck(UserSchema, "name", many=True) + + s = FlatBlogSchema() + data = s.dump(blog) + assert data["user"] == blog.user.name + for i, name in enumerate(data["collaborators"]): + assert name == blog.collaborators[i].name + + def test_dump_instanced_pluck(self, blog): + with pytest.warns(FieldConfigurationWarning): + + class FlatBlogSchema(Schema): + user = fields.Pluck(UserSchema(), "name") + collaborators = fields.Pluck(UserSchema(), "name", many=True) s = FlatBlogSchema() data = s.dump(blog) @@ -2177,6 +2190,22 @@ class FlatBlogSchema(Schema): for i, name in enumerate(data["collaborators"]): assert name == blog.collaborators[i].name + def test_load_pluck(self): + in_data = { + "user": "Doris", + "collaborators": ["Mick", "Keith"], + } + + class FlatBlogSchema(Schema): + user = fields.Pluck(UserSchema, "name") + collaborators = fields.Pluck(UserSchema, "name", many=True) + + s = FlatBlogSchema() + blog = s.load(in_data) + assert in_data["user"] == blog["user"].name + for i, collab in enumerate(blog["collaborators"]): + assert in_data["collaborators"][i] == collab.name + def test_pluck_none(self, blog): class FlatBlogSchema(Schema): user = fields.Pluck(UserSchema, "name")