Make unknown propagation override child schemas

In order to simplify, the value of `unknown` will not be respected
in child schemas if `PROPAGATE` is used. This removes any need for the
`auto_unknown` tracking, so there's less complexity added to schemas
in order to implement this behavior.

Adjust tests and changelog to match.

Also make minor tweaks to ensure UnknownParam usage is consistent and
keep the diff with the dev branch smaller.

Author: sirosen

CHANGELOG.rst

@@ -8,24 +8,16 @@ Features:
 
 - The behavior of the ``unknown`` option can be further customized with a new
   value, ``PROPAGATE``. If ``unknown=EXCLUDE | PROPAGATE`` is set, then the
-  value of ``unknown=EXCLUDE | PROPAGATE`` will be passed to any nested
-  schemas which do not explicitly set ``unknown`` in ``Nested`` or schema
-  options. This works for ``INCLUDE | PROPAGATE`` and ``RAISE | PROPAGATE`` as
-  well.
+  value of ``unknown=EXCLUDE | PROPAGATE`` will be passed to any nested schemas.
+  This works for ``INCLUDE | PROPAGATE`` and ``RAISE | PROPAGATE`` as well.
   (:issue:`1490`, :issue:`1428`)
-  Thanks :user:`lmignon` and :user:`mahenzon`.
 
 .. note::
 
-  When a schema is being loaded with ``unknown=... | PROPAGATE``, you can still
-  set ``unknown`` explicitly on child schemas. However, be aware that such a
-  value may turn off propagation at that point in the schema hierarchy.
-
-  For example, a schema which specifies ``unknown=EXCLUDE`` will set
-  ``EXCLUDE`` for itself. But because the value is ``EXCLUDE`` rather than
-  ``EXCLUDE | PROPAGATE``, that setting will not be propagated to its
-  children, even if there is a parent schema which sets
-  ``unknown=EXCLUDE | PROPAGATE``.
+  When a schema is being loaded with ``unknown=... | PROPAGATE``, this will
+  override any values set for ``unknown`` in child schemas. Therefore,
+  ``PROPAGATE`` should only be used in cases in which you want to change
+  the behavior of an entire schema heirarchy.
 
 3.7.0 (2020-07-08)
 ******************

src/marshmallow/base.py

@@ -38,9 +38,7 @@ def dump(self, obj, *, many: bool = None):
     def dumps(self, obj, *, many: bool = None):
         raise NotImplementedError
 
-    def load(
-        self, data, *, many: bool = None, partial=None, unknown=None
-    ):
+    def load(self, data, *, many: bool = None, partial=None, unknown=None):
         raise NotImplementedError
 
     def loads(

src/marshmallow/fields.py

@@ -494,7 +494,7 @@ def __init__(
         self.only = only
         self.exclude = exclude
         self.many = many
-        self.unknown = UnknownParam.parse_if_str(unknown) if unknown else None
+        self.unknown = UnknownParam.parse_if_str(unknown)
         self._schema = None  # Cached Schema instance
         super().__init__(default=default, **kwargs)
 
@@ -575,7 +575,7 @@ def _test_collection(self, value):
     def _load(self, value, data, partial=None, unknown=None):
         try:
             valid_data = self.schema.load(
-                value, unknown=unknown if unknown else self.unknown, partial=partial,
+                value, unknown=unknown or self.unknown, partial=partial,
             )
         except ValidationError as error:
             raise ValidationError(
@@ -593,18 +593,7 @@ def _deserialize(self, value, attr, data, partial=None, unknown=None, **kwargs):
             Add ``partial`` parameter.
         """
         self._test_collection(value)
-        # check if self.unknown or self.schema.unknown is set
-        # however, we should only respect `self.schema.unknown` if
-        # `auto_unknown` is False, meaning that it was set explicitly on the
-        # schema class or instance
-        explicit_unknown = (
-            self.unknown
-            if self.unknown
-            else (self.schema.unknown if not self.schema.auto_unknown else None)
-        )
-        if explicit_unknown:
-            unknown = explicit_unknown
-        return self._load(value, data, partial=partial, unknown=unknown,)
+        return self._load(value, data, partial=partial, unknown=unknown)
 
 
 class Pluck(Nested):

src/marshmallow/schema.py

@@ -228,15 +228,7 @@ def __init__(self, meta, ordered: bool = False):
         self.include = getattr(meta, "include", {})
         self.load_only = getattr(meta, "load_only", ())
         self.dump_only = getattr(meta, "dump_only", ())
-        # self.unknown defaults to "RAISE", but note whether it was explicit or
-        # not, so that when we're handling "propagate" we can decide
-        # whether or not to propagate based on whether or not it was set
-        # explicitly
-        self.unknown = getattr(meta, "unknown", None)
-        self.auto_unknown = self.unknown is None
-        if self.auto_unknown:
-            self.unknown = RAISE
-        self.unknown = UnknownParam.parse_if_str(self.unknown)
+        self.unknown = UnknownParam.parse_if_str(getattr(meta, "unknown", RAISE))
         self.register = getattr(meta, "register", True)
 
 
@@ -397,14 +389,7 @@ def __init__(
         self.load_only = set(load_only) or set(self.opts.load_only)
         self.dump_only = set(dump_only) or set(self.opts.dump_only)
         self.partial = partial
-        self.unknown = (
-            UnknownParam.parse_if_str(unknown)
-            if unknown is not None
-            else self.opts.unknown
-        )
-        # if unknown was not set explicitly AND self.opts.auto_unknown is true,
-        # then the value should be considered "automatic"
-        self.auto_unknown = (not unknown) and self.opts.auto_unknown
+        self.unknown = UnknownParam.parse_if_str(unknown) or self.opts.unknown
         self.context = context or {}
         self._normalize_nested_options()
         #: Dictionary mapping field_names -> :class:`Field` objects
@@ -775,7 +760,7 @@ def loads(
             if invalid data are passed.
         """
         data = self.opts.render_module.loads(json_data, **kwargs)
-        return self.load(data, many=many, partial=partial, unknown=unknown,)
+        return self.load(data, many=many, partial=partial, unknown=unknown)
 
     def _run_validator(
         self,
@@ -857,9 +842,7 @@ def _do_load(
         error_store = ErrorStore()
         errors = {}  # type: typing.Dict[str, typing.List[str]]
         many = self.many if many is None else bool(many)
-        unknown = UnknownParam.parse_if_str(
-            unknown if unknown is not None else self.unknown
-        )
+        unknown = UnknownParam.parse_if_str(unknown or self.unknown)
         if partial is None:
             partial = self.partial
         # Run preprocessors

src/marshmallow/utils.py

@@ -51,7 +51,10 @@ def __repr__(self):
 
     @classmethod
     def parse_if_str(cls, value):
-        """Given a string or UnknownParam, convert to an UnknownParam"""
+        """Given a string or UnknownParam, convert to an UnknownParam
+
+        Preserves None, which is important for making sure that it can be used
+        blindly on `unknown` which may be a user-supplied value or a default"""
         if isinstance(value, str):
             return cls(value)
         return value

tests/test_schema.py

@@ -2894,12 +2894,11 @@ class DefinitelyUniqueSchema(Schema):
     assert SchemaClass is DefinitelyUniqueSchema
 
 
-def test_propagate_unknown_stops_at_explicit_value_for_nested():
-    # PROPAGATE should traverse any "auto_unknown" values and
-    # replace them with the "unknown" value from the parent context (schema or
-    # load arguments)
-    # this test makes sure that it stops when a nested field or schema has
-    # "unknown" set explicitly (so auto_unknown=False)
+def test_propagate_unknown_overrides_explicit_value_for_nested():
+    # PROPAGATE should traverse any schemas and replace them with the
+    # "unknown" value from the parent context (schema or load arguments)
+    # this test makes sure that it takes precedence when a nested field
+    # or schema has "unknown" set explicitly
 
     class Bottom(Schema):
         x = fields.Str()
@@ -2923,14 +2922,13 @@ class Top(Schema):
     assert result == {
         "x": "hi",
         "y": "bye",
-        "child": {"x": "hi", "y": "bye", "child": {"x": "hi"}},
+        "child": {"x": "hi", "y": "bye", "child": {"x": "hi", "y": "bye"}},
     }
 
 
-def test_propagate_unknown_stops_at_explicit_value_for_meta():
-    # this is the same as the above test of unknown propagation stopping where
-    # auto_unknown=False, but it checks that this applies when `unknown` is set
-    # by means of `Meta`
+def test_propagate_unknown_overrides_explicit_value_for_meta():
+    # this is the same as the above test of unknown propagation, but it checks that
+    # this applies when `unknown` is set by means of `Meta` as well
 
     class Bottom(Schema):
         x = fields.Str()
@@ -2939,25 +2937,21 @@ class Middle(Schema):
         x = fields.Str()
         child = fields.Nested(Bottom)
 
-        # set unknown explicitly here, so auto_unknown will be
-        # false going into Bottom, and also set propagate to make it propagate
-        # in this case
         class Meta:
-            unknown = EXCLUDE | PROPAGATE
+            unknown = EXCLUDE
 
     class Top(Schema):
         x = fields.Str()
         child = fields.Nested(Middle)
 
-    # sanity-check that auto-unknown is being set correctly
-    assert Top().auto_unknown
-    assert not Top(unknown=INCLUDE | PROPAGATE).auto_unknown
-    assert not Middle().auto_unknown
-
     data = {
         "x": "hi",
         "y": "bye",
         "child": {"x": "hi", "y": "bye", "child": {"x": "hi", "y": "bye"}},
     }
     result = Top(unknown=INCLUDE | PROPAGATE).load(data)
-    assert result == {"x": "hi", "y": "bye", "child": {"x": "hi", "child": {"x": "hi"}}}
+    assert result == {
+        "x": "hi",
+        "y": "bye",
+        "child": {"x": "hi", "y": "bye", "child": {"x": "hi", "y": "bye"}},
+    }