Skip to content

Commit

Permalink
5.x: propagate null_check to maps in lists (#1128)
Browse files Browse the repository at this point in the history
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.

Backport of #1127 to 5.x branch.
  • Loading branch information
ikonst authored Dec 8, 2022
1 parent d2037b8 commit adc1577
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
jobs:
test:

runs-on: ubuntu-latest
runs-on: ubuntu-20.04
strategy:
matrix:
python-version: ['3.6', '3.7', '3.8', 'pypy-3.6']
Expand Down
5 changes: 5 additions & 0 deletions docs/release_notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
Release Notes
=============

v5.3.4
----------
* Make serialization :code:`null_check=False` propagate to maps inside lists (#1128).


v5.3.3
----------
* Fix :py:class:`~pynamodb.pagination.PageIterator` and :py:class:`~pynamodb.pagination.ResultIterator`
Expand Down
2 changes: 1 addition & 1 deletion pynamodb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
"""
__author__ = 'Jharrod LaFon'
__license__ = 'MIT'
__version__ = '5.3.3'
__version__ = '5.3.4'
9 changes: 6 additions & 3 deletions pynamodb/attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def _container_serialize(self, null_check: bool = True) -> Dict[str, Dict[str, A
raise

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)
Expand Down Expand Up @@ -1123,7 +1123,7 @@ 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.
"""
Expand All @@ -1133,7 +1133,10 @@ def serialize(self, values):
if self.element_type and v is not None and not isinstance(attr_class, self.element_type):
raise ValueError("List elements must be of type: {}".format(self.element_type.__name__))
attr_type = attr_class.attr_type
attr_value = attr_class.serialize(v)
if isinstance(attr_class, (ListAttribute, MapAttribute)):
attr_value = attr_class.serialize(v, null_check=null_check)
else:
attr_value = attr_class.serialize(v)
if attr_value is None:
# When attribute values serialize to "None" (e.g. empty sets) we store {"NULL": True} in DynamoDB.
attr_type = NULL
Expand Down
72 changes: 46 additions & 26 deletions tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,29 +391,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):
Expand Down Expand Up @@ -2911,41 +2911,61 @@ 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=False)

# now let's nullify an attribute a few levels deep to test that `null_check` propagates
item.left.left.left.value = None
item.serialize(null_check=False)

# now with null check
with pytest.raises(Exception, match="Attribute '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 '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,
Expand Down Expand Up @@ -3401,8 +3421,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)
Expand Down

0 comments on commit adc1577

Please sign in to comment.