diff --git a/docs/changelog.rst b/docs/changelog.rst index 1e223aa1..33578f01 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,7 @@ Development =========== - (Fill this out as you fix issues and develop your features). - Fix `_cls` that is not set properly in Document constructor (regression) #1950 +- Fix bug in _delta method - Update of a ListField depends on an unrelated dynamic field update #1733 - Remove deprecated `save()` method and used `insert_one()` #1899 ================= diff --git a/mongoengine/base/document.py b/mongoengine/base/document.py index 11b3a49a..e44ec2c9 100644 --- a/mongoengine/base/document.py +++ b/mongoengine/base/document.py @@ -582,7 +582,6 @@ class BaseDocument(object): set_fields = self._get_changed_fields() unset_data = {} - parts = [] if hasattr(self, '_changed_fields'): set_data = {} # Fetch each set item from its path @@ -592,15 +591,13 @@ class BaseDocument(object): new_path = [] for p in parts: if isinstance(d, (ObjectId, DBRef)): + # Don't dig in the references break - elif isinstance(d, list) and p.lstrip('-').isdigit(): - if p[0] == '-': - p = str(len(d) + int(p)) - try: - d = d[int(p)] - except IndexError: - d = None + elif isinstance(d, list) and p.isdigit(): + # An item of a list (identified by its index) is updated + d = d[int(p)] elif hasattr(d, 'get'): + # dict-like (dict, embedded document) d = d.get(p) new_path.append(p) path = '.'.join(new_path) @@ -612,26 +609,26 @@ class BaseDocument(object): # Determine if any changed items were actually unset. for path, value in set_data.items(): - if value or isinstance(value, (numbers.Number, bool)): + if value or isinstance(value, (numbers.Number, bool)): # Account for 0 and True that are truthy continue - # If we've set a value that ain't the default value don't unset it. - default = None + parts = path.split('.') + if (self._dynamic and len(parts) and parts[0] in self._dynamic_fields): del set_data[path] unset_data[path] = 1 continue - elif path in self._fields: + + # If we've set a value that ain't the default value don't unset it. + default = None + if path in self._fields: default = self._fields[path].default else: # Perform a full lookup for lists / embedded lookups d = self - parts = path.split('.') db_field_name = parts.pop() for p in parts: - if isinstance(d, list) and p.lstrip('-').isdigit(): - if p[0] == '-': - p = str(len(d) + int(p)) + if isinstance(d, list) and p.isdigit(): d = d[int(p)] elif (hasattr(d, '__getattribute__') and not isinstance(d, dict)): @@ -649,10 +646,9 @@ class BaseDocument(object): default = None if default is not None: - if callable(default): - default = default() + default = default() if callable(default) else default - if default != value: + if value != default: continue del set_data[path] diff --git a/tests/fields/fields.py b/tests/fields/fields.py index ccf67031..a1b586ce 100644 --- a/tests/fields/fields.py +++ b/tests/fields/fields.py @@ -186,6 +186,31 @@ class FieldTest(MongoDBTestCase): data_to_be_saved = sorted(person.to_mongo().keys()) self.assertEqual(data_to_be_saved, ['age', 'created', 'userid']) + def test_default_value_is_not_used_when_changing_value_to_empty_list_for_strict_doc(self): + """List field with default can be set to the empty list (strict)""" + # Issue #1733 + class Doc(Document): + x = ListField(IntField(), default=lambda: [42]) + + doc = Doc(x=[1]).save() + doc.x = [] + doc.save() + reloaded = Doc.objects.get(id=doc.id) + self.assertEqual(reloaded.x, []) + + def test_default_value_is_not_used_when_changing_value_to_empty_list_for_dyn_doc(self): + """List field with default can be set to the empty list (dynamic)""" + # Issue #1733 + class Doc(DynamicDocument): + x = ListField(IntField(), default=lambda: [42]) + + doc = Doc(x=[1]).save() + doc.x = [] + doc.y = 2 # Was triggering the bug + doc.save() + reloaded = Doc.objects.get(id=doc.id) + self.assertEqual(reloaded.x, []) + def test_default_values_when_deleting_value(self): """Ensure that default field values are used after non-default values are explicitly deleted.