Propagate null_check to maps in lists (pynamodb#1127)

ikonst · web-flow · commit 34ebe2efbbf6 · 2022-12-07T19:52:16.000-05:00
When calling model.serialize(null_check=False), we were propagating null_check=False to maps (and maps nested in maps), but not maps nested in lists.
diff --git a/pynamodb/attributes.py b/pynamodb/attributes.py
@@ -396,7 +396,7 @@ def _container_serialize(self, null_check: bool = True) -> Dict[str, Dict[str, A
                     raise ValueError("Attribute '{}' is not correctly typed".format(name))
 
                 if value is not None:
-                    if isinstance(attr, MapAttribute):
+                    if isinstance(attr, (ListAttribute, MapAttribute)):
                         attr_value = attr.serialize(value, null_check=null_check)
                     else:
                         attr_value = attr.serialize(value)
@@ -1243,18 +1243,21 @@ def __init__(
                 raise ValueError("'of' must be a subclass of Attribute")
             self.element_type = of
 
-    def serialize(self, values):
+    def serialize(self, values, *, null_check: bool = True):
         """
         Encode the given list of objects into a list of AttributeValue types.
         """
         rval = []
-        for idx, v in enumerate(values):
-            attr_class = self._get_serialize_class(v)
-            if self.element_type and v is not None and not isinstance(attr_class, self.element_type):
+        for idx, value in enumerate(values):
+            attr = self._get_serialize_class(value)
+            if self.element_type and value is not None and not isinstance(attr, self.element_type):
                 raise ValueError("List elements must be of type: {}".format(self.element_type.__name__))
-            attr_type = attr_class.attr_type
+            attr_type = attr.attr_type
             try:
-                attr_value = attr_class.serialize(v)
+                if isinstance(attr, (ListAttribute, MapAttribute)):
+                    attr_value = attr.serialize(value, null_check=null_check)
+                else:
+                    attr_value = attr.serialize(value)
             except AttributeNullError as e:
                 e.prepend_path(f'[{idx}]')
                 raise
diff --git a/tests/test_model.py b/tests/test_model.py
@@ -390,29 +390,29 @@ class Meta:
     is_human = BooleanAttribute()
 
 
-class TreeLeaf2(MapAttribute):
+class TreeLeaf(MapAttribute):
     value = NumberAttribute()
 
 
-class TreeLeaf1(MapAttribute):
+class TreeNode2(MapAttribute):
     value = NumberAttribute()
-    left = TreeLeaf2()
-    right = TreeLeaf2()
+    left = TreeLeaf()
+    right = TreeLeaf()
 
 
-class TreeLeaf(MapAttribute):
+class TreeNode1(MapAttribute):
     value = NumberAttribute()
-    left = TreeLeaf1()
-    right = TreeLeaf1()
+    left = TreeNode2()
+    right = TreeNode2()
 
 
 class TreeModel(Model):
     class Meta:
         table_name = 'TreeModelTable'
 
     tree_key = UnicodeAttribute(hash_key=True)
-    left = TreeLeaf()
-    right = TreeLeaf()
+    left = TreeNode1()
+    right = TreeNode1()
 
 
 class ExplicitRawMapModel(Model):
@@ -2822,34 +2822,40 @@ def test_deserializing_new_style_bool_true_works(self):
             self.assertTrue(item.is_human)
 
     def test_serializing_map_with_null_check(self):
-        item = TreeModel(
+        class TreeModelWithList(TreeModel):
+            leaves = ListAttribute(of=TreeLeaf)
+
+        item = TreeModelWithList(
             tree_key='test',
-            left=TreeLeaf(
+            left=TreeNode1(
                 value=42,
-                left=TreeLeaf1(
+                left=TreeNode2(
                     value=42,
-                    left=TreeLeaf2(value=42),
-                    right=TreeLeaf2(value=42),
+                    left=TreeLeaf(value=42),
+                    right=TreeLeaf(value=42),
                 ),
-                right=TreeLeaf1(
+                right=TreeNode2(
                     value=42,
-                    left=TreeLeaf2(value=42),
-                    right=TreeLeaf2(value=42),
+                    left=TreeLeaf(value=42),
+                    right=TreeLeaf(value=42),
                 ),
             ),
-            right=TreeLeaf(
+            right=TreeNode1(
                 value=42,
-                left=TreeLeaf1(
+                left=TreeNode2(
                     value=42,
-                    left=TreeLeaf2(value=42),
-                    right=TreeLeaf2(value=42),
+                    left=TreeLeaf(value=42),
+                    right=TreeLeaf(value=42),
                 ),
-                right=TreeLeaf1(
+                right=TreeNode2(
                     value=42,
-                    left=TreeLeaf2(value=42),
-                    right=TreeLeaf2(value=42),
+                    left=TreeLeaf(value=42),
+                    right=TreeLeaf(value=42),
                 ),
             ),
+            leaves=[
+                TreeLeaf(value=42),
+            ],
         )
         item.serialize(null_check=True)
 
@@ -2861,6 +2867,16 @@ def test_serializing_map_with_null_check(self):
         with pytest.raises(Exception, match="Attribute 'left.left.left.value' cannot be None"):
             item.serialize(null_check=True)
 
+        # now let's nullify an attribute of a map in a list to test that `null_check` propagates
+        item.left.left.left.value = 42
+        item.leaves[0].value = None
+        item.serialize(null_check=False)
+
+        # now with null check
+        with pytest.raises(Exception, match=r"Attribute 'leaves.\[0\].value' cannot be None"):
+            item.serialize(null_check=True)
+
+
     def test_deserializing_map_four_layers_deep_works(self):
         fake_db = self.database_mocker(TreeModel,
                                        TREE_MODEL_TABLE_DATA,
@@ -3317,8 +3333,8 @@ def test_subclassed_map_attribute_with_map_attributes_member_with_dict_init(self
     def test_subclassed_map_attribute_with_map_attribute_member_with_initialized_instance_init(self):
         left = self._get_bin_tree()
         right = self._get_bin_tree(multiplier=2)
-        left_instance = TreeLeaf(**left)
-        right_instance = TreeLeaf(**right)
+        left_instance = TreeNode1(**left)
+        right_instance = TreeNode1(**right)
         actual = TreeModel(tree_key='key', left=left_instance, right=right_instance)
         self.assertEqual(actual.left.left.right.value, left_instance.left.right.value)
         self.assertEqual(actual.left.left.value, left_instance.left.value)
