Repository: marshmallow-code/marshmallow

Issue:
Unenveloping None values doesn't work as expected
Trying to unenvelope serialized data when the contained data is None continues deserialization with the original data. This makes it impossible to deserialize nested fields with enveloped data where None is a valid value. (Related: http://jsonapi.org/format/)

Example (let's pretend our system allows for homeless employees):
https://gist.github.com/tdevelioglu/8e433355494ba022f48f
(I expect a deserialized Resource object, with an attribute 'address' with value 'None')


Discussion:

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 50d5f9d4..d6aedd4a 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,19 +6,25 @@ Changelog
 
 Features:
 
-* Clean up code for schema hooks (:issue:`814`). Thanks :user:`taion`.
-* Minor performance improvement from simplifying ``utils.get_value`` (:issue:`811`). Thanks again :user:`taion`.
-* Add ``require_tld`` argument to ``fields.URL`` (:issue:`749`). Thanks
+- Clean up code for schema hooks (:issue:`814`). Thanks :user:`taion`.
+- Minor performance improvement from simplifying ``utils.get_value`` (:issue:`811`). Thanks again :user:`taion`.
+- Add ``require_tld`` argument to ``fields.URL`` (:issue:`749`). Thanks
   :user:`DenerKup` for reporting and thanks :user:`surik00` for the PR.
-* ``fields.UUID`` deserializes ``bytes`` strings using ``UUID(bytes=b'...')`` (:issue:`625`).
+- ``fields.UUID`` deserializes ``bytes`` strings using ``UUID(bytes=b'...')`` (:issue:`625`).
   Thanks :user:`JeffBerger` for the suggestion and the PR.
 
 Bug fixes:
 
-* Fields nested within ``Dict`` correctly inherit context from their
+- Fields nested within ``Dict`` correctly inherit context from their
   parent schema (:issue:`820`). Thanks :user:`RosanneZe` for reporting
   and :user:`deckar01` for the PR.
-* Includes bug fix from 2.15.3.
+- Includes bug fix from 2.15.3.
+
+Other changes:
+
+- *Backwards-incompatible*: Pre/Post-processors must return modified data.
+  ``None`` does not mean data was mutated (:issue:`347`). Thanks
+  :user:`tdevelioglu` for reporting.
 
 3.0.0b10 (2018-05-10)
 +++++++++++++++++++++
diff --git a/docs/upgrading.rst b/docs/upgrading.rst
index 7cf5c67c..b013a526 100644
--- a/docs/upgrading.rst
+++ b/docs/upgrading.rst
@@ -456,6 +456,14 @@ It is not possible to specify a different key for serialization and deserializat
 Also, when ``data_key`` is specified on a field, only ``data_key`` is checked in the input data. In marshmallow 2.x the field name is checked if ``load_from`` is missing from the input data.
 
 
+Pre/Post-processors must return modified data
+*********************************************
+
+In marshmallow 2.x, ``None`` returned by a pre or post-processor is interpreted as "the data was mutated". In marshmallow 3.x, the return value is considered as processed data even if it is ``None``.
+
+Processors that mutate the data should be updated to also return it.
+
+
 Upgrading to 2.3
 ++++++++++++++++
 
diff --git a/marshmallow/schema.py b/marshmallow/schema.py
index c634bc26..09e853d2 100644
--- a/marshmallow/schema.py
+++ b/marshmallow/schema.py
@@ -927,20 +927,20 @@ class BaseSchema(base.SchemaABC):
 
             if pass_many:
                 if pass_original:
-                    data = utils.if_none(processor(data, many, original_data), data)
+                    data = processor(data, many, original_data)
                 else:
-                    data = utils.if_none(processor(data, many), data)
+                    data = processor(data, many)
             elif many:
                 if pass_original:
-                    data = [utils.if_none(processor(item, original), item)
+                    data = [processor(item, original)
                             for item, original in zip(data, original_data)]
                 else:
-                    data = [utils.if_none(processor(item), item) for item in data]
+                    data = [processor(item) for item in data]
             else:
                 if pass_original:
-                    data = utils.if_none(processor(data, original_data), data)
+                    data = processor(data, original_data)
                 else:
-                    data = utils.if_none(processor(data), data)
+                    data = processor(data)
         return data
 
 
diff --git a/marshmallow/utils.py b/marshmallow/utils.py
index 91423108..592d4016 100755
--- a/marshmallow/utils.py
+++ b/marshmallow/utils.py
@@ -398,7 +398,3 @@ def get_func_args(func):
         return _signature(func)
     # Callable class
     return _signature(func.__call__)
-
-
-def if_none(value, default):
-    return value if value is not None else default


Test patch:
diff --git a/tests/test_decorators.py b/tests/test_decorators.py
index 41a2d44a..58b1a298 100644
--- a/tests/test_decorators.py
+++ b/tests/test_decorators.py
@@ -22,24 +22,25 @@ def test_decorated_processors():
 
         value = fields.Integer(as_string=True)
 
-        # Implicit default raw, pre dump, static method, return modified item.
+        # Implicit default raw, pre dump, static method.
         @pre_dump
         def increment_value(self, item):
             item['value'] += 1
             return item
 
-        # Implicit default raw, post dump, class method, modify in place.
+        # Implicit default raw, post dump, class method.
         @post_dump
         def add_tag(self, item):
             item['value'] = self.TAG + item['value']
+            return item
 
-        # Explicitly raw, post dump, instance method, return modified item.
+        # Explicitly raw, post dump, instance method.
         @post_dump(pass_many=True)
         def add_envelope(self, data, many):
             key = self.get_envelope_key(many)
             return {key: data}
 
-        # Explicitly raw, pre load, instance method, return modified item.
+        # Explicitly raw, pre load, instance method.
         @pre_load(pass_many=True)
         def remove_envelope(self, data, many):
             key = self.get_envelope_key(many)
@@ -49,15 +50,17 @@ def test_decorated_processors():
         def get_envelope_key(many):
             return 'data' if many else 'datum'
 
-        # Explicitly not raw, pre load, instance method, modify in place.
+        # Explicitly not raw, pre load, instance method.
         @pre_load(pass_many=False)
         def remove_tag(self, item):
             item['value'] = item['value'][len(self.TAG):]
+            return item
 
-        # Explicit default raw, post load, instance method, modify in place.
+        # Explicit default raw, post load, instance method.
         @post_load()
         def decrement_value(self, item):
             item['value'] -= 1
+            return item
 
     schema = ExampleSchema()
 
@@ -75,9 +78,44 @@ def test_decorated_processors():
     items_loaded = schema.load(items_dumped, many=True)
     assert items_loaded == make_items()
 
+
+# Regression test for https://github.com/marshmallow-code/marshmallow/issues/347
+def test_decorated_processor_returning_none():
+    class PostSchema(Schema):
+        value = fields.Integer()
+
+        @post_load
+        def load_none(self, item):
+            return None
+
+        @post_dump
+        def dump_none(self, item):
+            return None
+
+    class PreSchema(Schema):
+        value = fields.Integer()
+
+        @pre_load
+        def load_none(self, item):
+            return None
+
+        @pre_dump
+        def dump_none(self, item):
+            return None
+
+    schema = PostSchema()
+    assert schema.dump({'value': 3}) is None
+    assert schema.load({'value': 3}) is None
+    schema = PreSchema()
+    assert schema.dump({'value': 3}) == {}
+    with pytest.raises(ValidationError) as excinfo:
+        schema.load({'value': 3})
+    assert excinfo.value.messages == {'_schema': ['Invalid input type.']}
+
+
 class TestPassOriginal:
 
-    def test_pass_original_single_no_mutation(self):
+    def test_pass_original_single(self):
         class MySchema(Schema):
             foo = fields.Field()
 
@@ -104,19 +142,6 @@ class TestPassOriginal:
         assert item_dumped['foo'] == 42
         assert item_dumped['_post_dump'] == 24
 
-    def test_pass_original_single_with_mutation(self):
-        class MySchema(Schema):
-            foo = fields.Field()
-
-            @post_load(pass_original=True)
-            def post_load(self, data, input_data):
-                data['_post_load'] = input_data['post_load']
-
-        schema = MySchema()
-        item_loaded = schema.load({'foo': 42, 'post_load': 24})
-        assert item_loaded['foo'] == 42
-        assert item_loaded['_post_load'] == 24
-
     def test_pass_original_many(self):
         class MySchema(Schema):
             foo = fields.Field()
@@ -218,6 +243,7 @@ def test_pre_dump_is_invoked_before_implicit_field_generation():
         @pre_dump
         def hook(s, data):
             data['generated_field'] = 7
+            return data
 
         class Meta:
             # Removing generated_field from here drops it from the output
@@ -574,7 +600,7 @@ def test_decorator_error_handling():
         @pre_load()
         def pre_load_error1(self, item):
             if item['foo'] != 0:
-                return
+                return item
             errors = {
                 'foo': ['preloadmsg1'],
                 'bar': ['preloadmsg2', 'preloadmsg3'],
@@ -584,13 +610,13 @@ def test_decorator_error_handling():
         @pre_load()
         def pre_load_error2(self, item):
             if item['foo'] != 4:
-                return
+                return item
             raise ValidationError('preloadmsg1', 'foo')
 
         @pre_load()
         def pre_load_error3(self, item):
             if item['foo'] != 8:
-                return
+                return item
             raise ValidationError('preloadmsg1')
 
         @post_load()
@@ -612,7 +638,7 @@ def test_decorator_error_handling():
         @pre_dump()
         def pre_dump_error1(self, item):
             if item['foo'] != 2:
-                return
+                return item
             errors = {
                 'foo': ['predumpmsg1'],
                 'bar': ['predumpmsg2', 'predumpmsg3'],
@@ -622,7 +648,7 @@ def test_decorator_error_handling():
         @pre_dump()
         def pre_dump_error2(self, item):
             if item['foo'] != 6:
-                return
+                return item
             raise ValidationError('predumpmsg1', 'foo')
 
         @post_dump()
@@ -638,7 +664,7 @@ def test_decorator_error_handling():
         @post_dump()
         def post_dump_error2(self, item):
             if item['foo'] != 7:
-                return
+                return item
             raise ValidationError('postdumpmsg1', 'foo')
 
     def make_item(foo, bar):