From d4350e7da425ea98a9122f2aa812730a0e4c704a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastien=20G=C3=A9rard?= Date: Sat, 10 Oct 2020 23:32:22 +0200 Subject: [PATCH 1/2] Fix for ListField that isnt detecting properly that item 0 is changed --- mongoengine/base/datastructures.py | 2 +- tests/document/test_delta.py | 50 ++++++++++++++++++------------ tests/document/test_instance.py | 7 +++-- tests/fields/test_dict_field.py | 2 +- tests/fields/test_fields.py | 4 +-- tests/fields/test_geo_fields.py | 4 +-- tests/test_datastructures.py | 6 +++- 7 files changed, 45 insertions(+), 30 deletions(-) diff --git a/mongoengine/base/datastructures.py b/mongoengine/base/datastructures.py index d08d4930..8c69cc73 100644 --- a/mongoengine/base/datastructures.py +++ b/mongoengine/base/datastructures.py @@ -179,7 +179,7 @@ class BaseList(list): def _mark_as_changed(self, key=None): if hasattr(self._instance, "_mark_as_changed"): - if key: + if key is not None: self._instance._mark_as_changed( "{}.{}".format(self._name, key % len(self)) ) diff --git a/tests/document/test_delta.py b/tests/document/test_delta.py index 2324211b..e7baaa23 100644 --- a/tests/document/test_delta.py +++ b/tests/document/test_delta.py @@ -29,7 +29,8 @@ class TestDelta(MongoDBTestCase): self.delta(Document) self.delta(DynamicDocument) - def delta(self, DocClass): + @staticmethod + def delta(DocClass): class Doc(DocClass): string_field = StringField() int_field = IntField() @@ -428,13 +429,20 @@ class TestDelta(MongoDBTestCase): assert doc.dict_field == {"hello": "world"} assert doc.list_field == ["1", 2, {"hello": "world"}] - def test_delta_recursive_db_field(self): + def test_delta_recursive_db_field_on_doc_and_embeddeddoc(self): self.delta_recursive_db_field(Document, EmbeddedDocument) + + def test_delta_recursive_db_field_on_doc_and_dynamicembeddeddoc(self): self.delta_recursive_db_field(Document, DynamicEmbeddedDocument) + + def test_delta_recursive_db_field_on_dynamicdoc_and_embeddeddoc(self): self.delta_recursive_db_field(DynamicDocument, EmbeddedDocument) + + def test_delta_recursive_db_field_on_dynamicdoc_and_dynamicembeddeddoc(self): self.delta_recursive_db_field(DynamicDocument, DynamicEmbeddedDocument) - def delta_recursive_db_field(self, DocClass, EmbeddedClass): + @staticmethod + def delta_recursive_db_field(DocClass, EmbeddedClass): class Embedded(EmbeddedClass): string_field = StringField(db_field="db_string_field") int_field = IntField(db_field="db_int_field") @@ -487,6 +495,7 @@ class TestDelta(MongoDBTestCase): doc = doc.reload(10) assert doc.embedded_field.dict_field == {} + assert doc._get_changed_fields() == [] doc.embedded_field.list_field = [] assert doc._get_changed_fields() == ["db_embedded_field.db_list_field"] assert doc.embedded_field._delta() == ({}, {"db_list_field": 1}) @@ -634,6 +643,7 @@ class TestDelta(MongoDBTestCase): doc.save() doc = doc.reload(10) + assert doc._delta() == ({}, {},) del doc.embedded_field.list_field[2].list_field assert doc._delta() == ( {}, @@ -732,12 +742,12 @@ class TestDelta(MongoDBTestCase): assert organization._get_changed_fields() == [] updates, removals = organization._delta() - assert {} == removals - assert {} == updates + assert removals == {} + assert updates == {} organization.employees.append(person) updates, removals = organization._delta() - assert {} == removals + assert removals == {} assert "employees" in updates def test_delta_with_dbref_false(self): @@ -749,12 +759,12 @@ class TestDelta(MongoDBTestCase): assert organization._get_changed_fields() == [] updates, removals = organization._delta() - assert {} == removals - assert {} == updates + assert removals == {} + assert updates == {} organization.employees.append(person) updates, removals = organization._delta() - assert {} == removals + assert removals == {} assert "employees" in updates def test_nested_nested_fields_mark_as_changed(self): @@ -775,11 +785,11 @@ class TestDelta(MongoDBTestCase): subdoc = mydoc.subs["a"]["b"] subdoc.name = "bar" - assert ["name"] == subdoc._get_changed_fields() - assert ["subs.a.b.name"] == mydoc._get_changed_fields() + assert subdoc._get_changed_fields() == ["name"] + assert mydoc._get_changed_fields() == ["subs.a.b.name"] mydoc._clear_changed_fields() - assert [] == mydoc._get_changed_fields() + assert mydoc._get_changed_fields() == [] def test_lower_level_mark_as_changed(self): class EmbeddedDoc(EmbeddedDocument): @@ -794,17 +804,17 @@ class TestDelta(MongoDBTestCase): mydoc = MyDoc.objects.first() mydoc.subs["a"] = EmbeddedDoc() - assert ["subs.a"] == mydoc._get_changed_fields() + assert mydoc._get_changed_fields() == ["subs.a"] subdoc = mydoc.subs["a"] subdoc.name = "bar" - assert ["name"] == subdoc._get_changed_fields() - assert ["subs.a"] == mydoc._get_changed_fields() + assert subdoc._get_changed_fields() == ["name"] + assert mydoc._get_changed_fields() == ["subs.a"] mydoc.save() mydoc._clear_changed_fields() - assert [] == mydoc._get_changed_fields() + assert mydoc._get_changed_fields() == [] def test_upper_level_mark_as_changed(self): class EmbeddedDoc(EmbeddedDocument): @@ -821,15 +831,15 @@ class TestDelta(MongoDBTestCase): subdoc = mydoc.subs["a"] subdoc.name = "bar" - assert ["name"] == subdoc._get_changed_fields() - assert ["subs.a.name"] == mydoc._get_changed_fields() + assert subdoc._get_changed_fields() == ["name"] + assert mydoc._get_changed_fields() == ["subs.a.name"] mydoc.subs["a"] = EmbeddedDoc() - assert ["subs.a"] == mydoc._get_changed_fields() + assert mydoc._get_changed_fields() == ["subs.a"] mydoc.save() mydoc._clear_changed_fields() - assert [] == mydoc._get_changed_fields() + assert mydoc._get_changed_fields() == [] def test_referenced_object_changed_attributes(self): """Ensures that when you save a new reference to a field, the referenced object isn't altered""" diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 993cc161..8d42d15b 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -188,7 +188,7 @@ class TestDocumentInstance(MongoDBTestCase): def test_queryset_resurrects_dropped_collection(self): self.Person.drop_collection() - assert [] == list(self.Person.objects()) + assert list(self.Person.objects()) == [] # Ensure works correctly with inhertited classes class Actor(self.Person): @@ -196,7 +196,7 @@ class TestDocumentInstance(MongoDBTestCase): Actor.objects() self.Person.drop_collection() - assert [] == list(Actor.objects()) + assert list(Actor.objects()) == [] def test_polymorphic_references(self): """Ensure that the correct subclasses are returned from a query @@ -578,7 +578,8 @@ class TestDocumentInstance(MongoDBTestCase): doc.embedded_field.list_field.append(1) doc.embedded_field.dict_field["woot"] = "woot" - assert doc._get_changed_fields() == [ + changed = doc._get_changed_fields() + assert changed == [ "list_field", "dict_field.woot", "embedded_field.list_field", diff --git a/tests/fields/test_dict_field.py b/tests/fields/test_dict_field.py index f423bf8b..12140916 100644 --- a/tests/fields/test_dict_field.py +++ b/tests/fields/test_dict_field.py @@ -113,7 +113,7 @@ class TestDictField(MongoDBTestCase): post.info.setdefault("authors", []) post.save() post.reload() - assert [] == post.info["authors"] + assert post.info["authors"] == [] def test_dictfield_dump_document(self): """Ensure a DictField can handle another document's dump.""" diff --git a/tests/fields/test_fields.py b/tests/fields/test_fields.py index 25ecb2e7..fe349d1e 100644 --- a/tests/fields/test_fields.py +++ b/tests/fields/test_fields.py @@ -1084,7 +1084,7 @@ class TestField(MongoDBTestCase): e = Simple().save() e.mapping = [] - assert [] == e._changed_fields + assert e._changed_fields == [] class Simple(Document): mapping = DictField() @@ -1093,7 +1093,7 @@ class TestField(MongoDBTestCase): e = Simple().save() e.mapping = {} - assert [] == e._changed_fields + assert e._changed_fields == [] def test_slice_marks_field_as_changed(self): class Simple(Document): diff --git a/tests/fields/test_geo_fields.py b/tests/fields/test_geo_fields.py index 1b912a4b..7618b3a0 100644 --- a/tests/fields/test_geo_fields.py +++ b/tests/fields/test_geo_fields.py @@ -381,7 +381,7 @@ class TestGeoField(MongoDBTestCase): meta = {"indexes": [[("location", "2dsphere"), ("datetime", 1)]]} - assert [] == Log._geo_indices() + assert Log._geo_indices() == [] Log.drop_collection() Log.ensure_indexes() @@ -401,7 +401,7 @@ class TestGeoField(MongoDBTestCase): "indexes": [{"fields": [("location", "2dsphere"), ("datetime", 1)]}] } - assert [] == Log._geo_indices() + assert Log._geo_indices() == [] Log.drop_collection() Log.ensure_indexes() diff --git a/tests/test_datastructures.py b/tests/test_datastructures.py index 6d432e32..f4b63f05 100644 --- a/tests/test_datastructures.py +++ b/tests/test_datastructures.py @@ -9,10 +9,14 @@ from mongoengine.base.datastructures import BaseDict, BaseList, StrictDict class DocumentStub(object): def __init__(self): self._changed_fields = [] + self._unset_fields = [] def _mark_as_changed(self, key): self._changed_fields.append(key) + def _mark_as_unset(self, key): + self._unset_fields.append(key) + class TestBaseDict: @staticmethod @@ -314,7 +318,7 @@ class TestBaseList: def test___setitem___item_0_calls_mark_as_changed(self): base_list = self._get_baselist([True]) base_list[0] = False - assert base_list._instance._changed_fields == ["my_name"] + assert base_list._instance._changed_fields == ["my_name.0"] assert base_list == [False] def test___setitem___item_1_calls_mark_as_changed(self): From 3adb67901b64e1e87668247cfd8183b4090a922b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastien=20G=C3=A9rard?= Date: Sun, 11 Oct 2020 00:53:46 +0200 Subject: [PATCH 2/2] update changelog for #2392 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 14dfb8d0..f616f4a6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,8 @@ Development This should have a negative impact on performance of count see Issue #2219 - Fix a bug that made the queryset drop the read_preference after clone(). - Fix the behavior of Doc.objects.limit(0) which should return all documents (similar to mongodb) #2311 +- Bug fix in ListField when updating the first item, it was saving the whole list, instead of + just replacing the first item (as it's usually done) #2392 Changes in 0.20.0 =================