From 88642eb0212d276eaa4efe0d605fc3af743158a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Perez?= Date: Tue, 23 Mar 2021 22:11:41 +0100 Subject: [PATCH 1/4] Fix deepcopy on EmbeddedDocument --- AUTHORS | 1 + docs/changelog.rst | 1 + mongoengine/document.py | 9 +++++++++ tests/document/test_instance.py | 9 +++++++++ 4 files changed, 20 insertions(+) diff --git a/AUTHORS b/AUTHORS index 0c7f6b46..64069d33 100644 --- a/AUTHORS +++ b/AUTHORS @@ -260,3 +260,4 @@ that much better: * Stankiewicz Mateusz (https://github.com/mas15) * Felix Schultheiß (https://github.com/felix-smashdocs) * Jan Stein (https://github.com/janste63) + * Timothé Perez (https://github.com/AchilleAsh) diff --git a/docs/changelog.rst b/docs/changelog.rst index 3f4252a5..737ec043 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,6 +13,7 @@ Changes in 0.23.1 =========== - Bug fix: ignore LazyReferenceFields when clearing _changed_fields #2484 - Improve connection doc #2481 +- Fix deepcopy on EmbeddedDocument Changes in 0.23.0 ================= diff --git a/mongoengine/document.py b/mongoengine/document.py index 0ba5db12..9be49368 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -99,6 +99,15 @@ class EmbeddedDocument(BaseDocument, metaclass=DocumentMetaclass): def __ne__(self, other): return not self.__eq__(other) + def __getstate__(self): + data = super().__getstate__() + data["_instance"] = self._instance + return data + + def __setstate__(self, state): + super().__setstate__(state) + self._instance = state["_instance"] + def to_mongo(self, *args, **kwargs): data = super().to_mongo(*args, **kwargs) diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 1469c9bb..77d51a46 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -3,6 +3,7 @@ import pickle import unittest import uuid import weakref +from copy import deepcopy from datetime import datetime import bson @@ -78,6 +79,14 @@ class TestDocumentInstance(MongoDBTestCase): else: assert field._instance == instance + def test_deepcopy(self): + """Ensure that the _instance attribute on EmbeddedDocument exists after a deepcopy""" + + doc = self.Job() + assert doc._instance is None + copied = deepcopy(doc) + assert copied._instance is None + def test_capped_collection(self): """Ensure that capped collections work properly.""" From c32b30873049192ca03f8b7993e069497e11b264 Mon Sep 17 00:00:00 2001 From: Bastien Gerard Date: Sun, 1 Aug 2021 14:12:27 +0200 Subject: [PATCH 2/4] minor renaming in test classes --- tests/document/test_instance.py | 30 +++++++++++++++--------------- tests/queryset/test_modify.py | 22 +++++++++++----------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 77d51a46..5d27a6cf 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -66,12 +66,12 @@ class TestDocumentInstance(MongoDBTestCase): for collection in list_collection_names(self.db): self.db.drop_collection(collection) - def assertDbEqual(self, docs): + def _assert_db_equal(self, docs): assert list(self.Person._get_collection().find().sort("id")) == sorted( docs, key=lambda doc: doc["_id"] ) - def assertHasInstance(self, field, instance): + def _assert_has_instance(self, field, instance): assert hasattr(field, "_instance") assert field._instance is not None if isinstance(field._instance, weakref.ProxyType): @@ -749,11 +749,11 @@ class TestDocumentInstance(MongoDBTestCase): Doc.drop_collection() doc = Doc(embedded_field=Embedded(string="Hi")) - self.assertHasInstance(doc.embedded_field, doc) + self._assert_has_instance(doc.embedded_field, doc) doc.save() doc = Doc.objects.get() - self.assertHasInstance(doc.embedded_field, doc) + self._assert_has_instance(doc.embedded_field, doc) def test_embedded_document_complex_instance(self): """Ensure that embedded documents in complex fields can reference @@ -768,11 +768,11 @@ class TestDocumentInstance(MongoDBTestCase): Doc.drop_collection() doc = Doc(embedded_field=[Embedded(string="Hi")]) - self.assertHasInstance(doc.embedded_field[0], doc) + self._assert_has_instance(doc.embedded_field[0], doc) doc.save() doc = Doc.objects.get() - self.assertHasInstance(doc.embedded_field[0], doc) + self._assert_has_instance(doc.embedded_field[0], doc) def test_embedded_document_complex_instance_no_use_db_field(self): """Ensure that use_db_field is propagated to list of Emb Docs.""" @@ -801,11 +801,11 @@ class TestDocumentInstance(MongoDBTestCase): acc = Account() acc.email = Email(email="test@example.com") - self.assertHasInstance(acc._data["email"], acc) + self._assert_has_instance(acc._data["email"], acc) acc.save() acc1 = Account.objects.first() - self.assertHasInstance(acc1._data["email"], acc1) + self._assert_has_instance(acc1._data["email"], acc1) def test_instance_is_set_on_setattr_on_embedded_document_list(self): class Email(EmbeddedDocument): @@ -817,11 +817,11 @@ class TestDocumentInstance(MongoDBTestCase): Account.drop_collection() acc = Account() acc.emails = [Email(email="test@example.com")] - self.assertHasInstance(acc._data["emails"][0], acc) + self._assert_has_instance(acc._data["emails"][0], acc) acc.save() acc1 = Account.objects.first() - self.assertHasInstance(acc1._data["emails"][0], acc1) + self._assert_has_instance(acc1._data["emails"][0], acc1) def test_save_checks_that_clean_is_called(self): class CustomError(Exception): @@ -930,7 +930,7 @@ class TestDocumentInstance(MongoDBTestCase): with pytest.raises(InvalidDocumentError): self.Person().modify(set__age=10) - self.assertDbEqual([dict(doc.to_mongo())]) + self._assert_db_equal([dict(doc.to_mongo())]) def test_modify_invalid_query(self): doc1 = self.Person(name="bob", age=10).save() @@ -940,7 +940,7 @@ class TestDocumentInstance(MongoDBTestCase): with pytest.raises(InvalidQueryError): doc1.modify({"id": doc2.id}, set__value=20) - self.assertDbEqual(docs) + self._assert_db_equal(docs) def test_modify_match_another_document(self): doc1 = self.Person(name="bob", age=10).save() @@ -950,7 +950,7 @@ class TestDocumentInstance(MongoDBTestCase): n_modified = doc1.modify({"name": doc2.name}, set__age=100) assert n_modified == 0 - self.assertDbEqual(docs) + self._assert_db_equal(docs) def test_modify_not_exists(self): doc1 = self.Person(name="bob", age=10).save() @@ -960,7 +960,7 @@ class TestDocumentInstance(MongoDBTestCase): n_modified = doc2.modify({"name": doc2.name}, set__age=100) assert n_modified == 0 - self.assertDbEqual(docs) + self._assert_db_equal(docs) def test_modify_update(self): other_doc = self.Person(name="bob", age=10).save() @@ -986,7 +986,7 @@ class TestDocumentInstance(MongoDBTestCase): assert doc.to_json() == doc_copy.to_json() assert doc._get_changed_fields() == [] - self.assertDbEqual([dict(other_doc.to_mongo()), dict(doc.to_mongo())]) + self._assert_db_equal([dict(other_doc.to_mongo()), dict(doc.to_mongo())]) def test_modify_with_positional_push(self): class Content(EmbeddedDocument): diff --git a/tests/queryset/test_modify.py b/tests/queryset/test_modify.py index 15e3af34..b96e05e6 100644 --- a/tests/queryset/test_modify.py +++ b/tests/queryset/test_modify.py @@ -19,7 +19,7 @@ class TestFindAndModify(unittest.TestCase): connect(db="mongoenginetest") Doc.drop_collection() - def assertDbEqual(self, docs): + def _assert_db_equal(self, docs): assert list(Doc._collection.find().sort("id")) == docs def test_modify(self): @@ -28,7 +28,7 @@ class TestFindAndModify(unittest.TestCase): old_doc = Doc.objects(id=1).modify(set__value=-1) assert old_doc.to_json() == doc.to_json() - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) def test_modify_with_new(self): Doc(id=0, value=0).save() @@ -37,18 +37,18 @@ class TestFindAndModify(unittest.TestCase): new_doc = Doc.objects(id=1).modify(set__value=-1, new=True) doc.value = -1 assert new_doc.to_json() == doc.to_json() - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) def test_modify_not_existing(self): Doc(id=0, value=0).save() assert Doc.objects(id=1).modify(set__value=-1) is None - self.assertDbEqual([{"_id": 0, "value": 0}]) + self._assert_db_equal([{"_id": 0, "value": 0}]) def test_modify_with_upsert(self): Doc(id=0, value=0).save() old_doc = Doc.objects(id=1).modify(set__value=1, upsert=True) assert old_doc is None - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": 1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": 1}]) def test_modify_with_upsert_existing(self): Doc(id=0, value=0).save() @@ -56,13 +56,13 @@ class TestFindAndModify(unittest.TestCase): old_doc = Doc.objects(id=1).modify(set__value=-1, upsert=True) assert old_doc.to_json() == doc.to_json() - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) def test_modify_with_upsert_with_new(self): Doc(id=0, value=0).save() new_doc = Doc.objects(id=1).modify(upsert=True, new=True, set__value=1) assert new_doc.to_mongo() == {"_id": 1, "value": 1} - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": 1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": 1}]) def test_modify_with_remove(self): Doc(id=0, value=0).save() @@ -70,12 +70,12 @@ class TestFindAndModify(unittest.TestCase): old_doc = Doc.objects(id=1).modify(remove=True) assert old_doc.to_json() == doc.to_json() - self.assertDbEqual([{"_id": 0, "value": 0}]) + self._assert_db_equal([{"_id": 0, "value": 0}]) def test_find_and_modify_with_remove_not_existing(self): Doc(id=0, value=0).save() assert Doc.objects(id=1).modify(remove=True) is None - self.assertDbEqual([{"_id": 0, "value": 0}]) + self._assert_db_equal([{"_id": 0, "value": 0}]) def test_modify_with_order_by(self): Doc(id=0, value=3).save() @@ -85,7 +85,7 @@ class TestFindAndModify(unittest.TestCase): old_doc = Doc.objects().order_by("-id").modify(set__value=-1) assert old_doc.to_json() == doc.to_json() - self.assertDbEqual( + self._assert_db_equal( [ {"_id": 0, "value": 3}, {"_id": 1, "value": 2}, @@ -100,7 +100,7 @@ class TestFindAndModify(unittest.TestCase): old_doc = Doc.objects(id=1).only("id").modify(set__value=-1) assert old_doc.to_mongo() == {"_id": 1} - self.assertDbEqual([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) + self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}]) def test_modify_with_push(self): class BlogPost(Document): From 5a41998edaaa0e9c9bb216739fae2efbfe632a6e Mon Sep 17 00:00:00 2001 From: Bastien Gerard Date: Mon, 2 Aug 2021 21:51:15 +0200 Subject: [PATCH 3/4] Updated recent bugfix with embedded DOc deepcopy so that it set _instance to None --- docs/changelog.rst | 1 + mongoengine/document.py | 4 +-- tests/document/test_instance.py | 9 ------ tests/fields/test_embedded_document_field.py | 31 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 737ec043..6d5f4a55 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,6 +8,7 @@ Development =========== - (Fill this out as you fix issues and develop your features). - EnumField improvements: now `choices` limits the values of an enum to allow +- Missing `._instance` field after deepcopy of EmbeddedField (#2202 Changes in 0.23.1 =========== diff --git a/mongoengine/document.py b/mongoengine/document.py index 9be49368..5f05b485 100644 --- a/mongoengine/document.py +++ b/mongoengine/document.py @@ -101,7 +101,7 @@ class EmbeddedDocument(BaseDocument, metaclass=DocumentMetaclass): def __getstate__(self): data = super().__getstate__() - data["_instance"] = self._instance + data["_instance"] = None return data def __setstate__(self, state): @@ -135,7 +135,7 @@ class Document(BaseDocument, metaclass=TopLevelDocumentMetaclass): create a specialised version of the document that will be stored in the same collection. To facilitate this behaviour a `_cls` field is added to documents (hidden though the MongoEngine interface). - To enable this behaviourset :attr:`allow_inheritance` to ``True`` in the + To enable this behaviour set :attr:`allow_inheritance` to ``True`` in the :attr:`meta` dictionary. A :class:`~mongoengine.Document` may use a **Capped Collection** by diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 5d27a6cf..cd8e9b1b 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -3,7 +3,6 @@ import pickle import unittest import uuid import weakref -from copy import deepcopy from datetime import datetime import bson @@ -79,14 +78,6 @@ class TestDocumentInstance(MongoDBTestCase): else: assert field._instance == instance - def test_deepcopy(self): - """Ensure that the _instance attribute on EmbeddedDocument exists after a deepcopy""" - - doc = self.Job() - assert doc._instance is None - copied = deepcopy(doc) - assert copied._instance is None - def test_capped_collection(self): """Ensure that capped collections work properly.""" diff --git a/tests/fields/test_embedded_document_field.py b/tests/fields/test_embedded_document_field.py index 0e9784ff..8aec8515 100644 --- a/tests/fields/test_embedded_document_field.py +++ b/tests/fields/test_embedded_document_field.py @@ -1,4 +1,7 @@ +from copy import deepcopy + import pytest +from bson import ObjectId from mongoengine import ( Document, @@ -9,6 +12,7 @@ from mongoengine import ( InvalidQueryError, ListField, LookUpError, + MapField, StringField, ValidationError, ) @@ -350,3 +354,30 @@ class TestGenericEmbeddedDocumentField(MongoDBTestCase): # Test existing attribute assert Person.objects(settings__base_foo="basefoo").first().id == p.id assert Person.objects(settings__sub_foo="subfoo").first().id == p.id + + def test_deepcopy_set__instance(self): + """Ensure that the _instance attribute on EmbeddedDocument exists after a deepcopy""" + + class Wallet(EmbeddedDocument): + money = IntField() + + class Person(Document): + wallet = EmbeddedDocumentField(Wallet) + wallet_map = MapField(EmbeddedDocumentField(Wallet)) + + # Test on fresh EmbeddedDoc + emb_doc = Wallet(money=1) + assert emb_doc._instance is None + copied_emb_doc = deepcopy(emb_doc) + assert copied_emb_doc._instance is None + + # Test on attached EmbeddedDoc + doc = Person( + id=ObjectId(), wallet=Wallet(money=2), wallet_map={"test": Wallet(money=2)} + ) + assert doc.wallet._instance == doc + copied_emb_doc = deepcopy(doc.wallet) + assert copied_emb_doc._instance == doc + + copied_map_emb_doc = deepcopy(doc.wallet_map) + assert copied_map_emb_doc["test"]._instance == doc From 66978aea672f7813d32a644f271f032c2b56cf42 Mon Sep 17 00:00:00 2001 From: Bastien Gerard Date: Mon, 2 Aug 2021 21:56:50 +0200 Subject: [PATCH 4/4] fix test and changelog --- docs/changelog.rst | 3 +-- tests/fields/test_embedded_document_field.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6d5f4a55..be2c38c3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,13 +8,12 @@ Development =========== - (Fill this out as you fix issues and develop your features). - EnumField improvements: now `choices` limits the values of an enum to allow -- Missing `._instance` field after deepcopy of EmbeddedField (#2202 +- Fix deepcopy of EmbeddedDocument #2202 Changes in 0.23.1 =========== - Bug fix: ignore LazyReferenceFields when clearing _changed_fields #2484 - Improve connection doc #2481 -- Fix deepcopy on EmbeddedDocument Changes in 0.23.0 ================= diff --git a/tests/fields/test_embedded_document_field.py b/tests/fields/test_embedded_document_field.py index 8aec8515..3b555cc6 100644 --- a/tests/fields/test_embedded_document_field.py +++ b/tests/fields/test_embedded_document_field.py @@ -377,7 +377,7 @@ class TestGenericEmbeddedDocumentField(MongoDBTestCase): ) assert doc.wallet._instance == doc copied_emb_doc = deepcopy(doc.wallet) - assert copied_emb_doc._instance == doc + assert copied_emb_doc._instance is None copied_map_emb_doc = deepcopy(doc.wallet_map) - assert copied_map_emb_doc["test"]._instance == doc + assert copied_map_emb_doc["test"]._instance is None