From 50882e5bb09b74faddfac3cb93afd278ea94ced2 Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Wed, 16 Oct 2019 09:49:40 -0400 Subject: [PATCH 1/6] Add failing test Test that __eq__ for EmbeddedDocuments with LazyReferenceFields works as expected. --- tests/document/test_instance.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 173e02f2..6ba6827e 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -3319,6 +3319,38 @@ class TestInstance(MongoDBTestCase): f1.ref # Dereferences lazily assert f1 == f2 + def test_embedded_document_equality_with_lazy_ref(self): + class Job(EmbeddedDocument): + boss = LazyReferenceField('Person') + + class Person(Document): + job = EmbeddedDocumentField(Job) + + Person.drop_collection() + + boss = Person() + worker = Person(job=Job(boss=boss)) + boss.save() + worker.save() + + worker1 = Person.objects.get(id=worker.id) + + # worker1.job should be equal to the job used originally to create the + # document. + self.assertEqual(worker1.job, worker.job) + + # worker1.job should be equal to a newly created Job EmbeddedDocument + # using either the Boss object or his ID. + self.assertEqual(worker1.job, Job(boss=boss)) + self.assertEqual(worker1.job, Job(boss=boss.id)) + + # The above equalities should also hold after worker1.job.boss has been + # fetch()ed. + worker1.job.boss.fetch() + self.assertEqual(worker1.job, worker.job) + self.assertEqual(worker1.job, Job(boss=boss)) + self.assertEqual(worker1.job, Job(boss=boss.id)) + def test_dbref_equality(self): class Test2(Document): name = StringField() From dc7b96a5691335e970b13fb30ef62426b126e2bd Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Wed, 16 Oct 2019 09:50:47 -0400 Subject: [PATCH 2/6] Make python value for LazyReferenceFields be a DBRef Previously, when reading a LazyReferenceField from the DB, it was stored internally in the parent document's _data field as an ObjectId. However, this meant that equality tests using an enclosing EmbeddedDocument would not return True when the EmbeddedDocument being compared to contained a DBRef or Document in _data. Enclosing Documents were largely unaffected because they look at the primary key for equality (which EmbeddedDocuments lack). This makes the internal Python representation of a LazyReferenceField (before the LazyReference itself has been constructed) a DBRef, using code identical to ReferenceField. --- mongoengine/fields.py | 9 +++++++++ tests/document/test_instance.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/mongoengine/fields.py b/mongoengine/fields.py index f8f527a3..0c29d1bc 100644 --- a/mongoengine/fields.py +++ b/mongoengine/fields.py @@ -2502,6 +2502,15 @@ class LazyReferenceField(BaseField): else: return pk + def to_python(self, value): + """Convert a MongoDB-compatible type to a Python type.""" + if not self.dbref and not isinstance( + value, (DBRef, Document, EmbeddedDocument) + ): + collection = self.document_type._get_collection_name() + value = DBRef(collection, self.document_type.id.to_python(value)) + return value + def validate(self, value): if isinstance(value, LazyReference): if value.collection != self.document_type._get_collection_name(): diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 6ba6827e..07376b4b 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -3321,7 +3321,7 @@ class TestInstance(MongoDBTestCase): def test_embedded_document_equality_with_lazy_ref(self): class Job(EmbeddedDocument): - boss = LazyReferenceField('Person') + boss = LazyReferenceField("Person") class Person(Document): job = EmbeddedDocumentField(Job) From 0d4e61d489a9264863cecdfed08fc9e67a74d03a Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Wed, 16 Oct 2019 10:01:19 -0400 Subject: [PATCH 3/6] Add daewok to AUTHORS per contributing guidelines --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index aa044bd2..374e2f7f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -253,3 +253,4 @@ that much better: * Gaurav Dadhania (https://github.com/GVRV) * Yurii Andrieiev (https://github.com/yandrieiev) * Filip Kucharczyk (https://github.com/Pacu2) + * Eric Timmons (https://github.com/daewok) From 68dc2925fbea13702fa23ced0afd786d77b2ca28 Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Sun, 15 Dec 2019 12:08:04 -0500 Subject: [PATCH 4/6] Add LazyReferenceField with dbref=True to embedded_document equality test --- tests/document/test_instance.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/document/test_instance.py b/tests/document/test_instance.py index 07376b4b..b899684f 100644 --- a/tests/document/test_instance.py +++ b/tests/document/test_instance.py @@ -3322,6 +3322,7 @@ class TestInstance(MongoDBTestCase): def test_embedded_document_equality_with_lazy_ref(self): class Job(EmbeddedDocument): boss = LazyReferenceField("Person") + boss_dbref = LazyReferenceField("Person", dbref=True) class Person(Document): job = EmbeddedDocumentField(Job) @@ -3329,7 +3330,7 @@ class TestInstance(MongoDBTestCase): Person.drop_collection() boss = Person() - worker = Person(job=Job(boss=boss)) + worker = Person(job=Job(boss=boss, boss_dbref=boss)) boss.save() worker.save() @@ -3341,15 +3342,15 @@ class TestInstance(MongoDBTestCase): # worker1.job should be equal to a newly created Job EmbeddedDocument # using either the Boss object or his ID. - self.assertEqual(worker1.job, Job(boss=boss)) - self.assertEqual(worker1.job, Job(boss=boss.id)) + self.assertEqual(worker1.job, Job(boss=boss, boss_dbref=boss)) + self.assertEqual(worker1.job, Job(boss=boss.id, boss_dbref=boss.id)) # The above equalities should also hold after worker1.job.boss has been # fetch()ed. worker1.job.boss.fetch() self.assertEqual(worker1.job, worker.job) - self.assertEqual(worker1.job, Job(boss=boss)) - self.assertEqual(worker1.job, Job(boss=boss.id)) + self.assertEqual(worker1.job, Job(boss=boss, boss_dbref=boss)) + self.assertEqual(worker1.job, Job(boss=boss.id, boss_dbref=boss.id)) def test_dbref_equality(self): class Test2(Document): From 329f030a41da4d93aaec1f3ccee634e898f7d289 Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Sun, 15 Dec 2019 20:15:13 -0500 Subject: [PATCH 5/6] Always store a DBRef, Document, or EmbeddedDocument in LazyReferenceField._data This is required to handle the case of equality tests on a LazyReferenceField with dbref=True when comparing against a field instantiated with an ObjectId. --- mongoengine/fields.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mongoengine/fields.py b/mongoengine/fields.py index 0c29d1bc..a385559d 100644 --- a/mongoengine/fields.py +++ b/mongoengine/fields.py @@ -2504,9 +2504,7 @@ class LazyReferenceField(BaseField): def to_python(self, value): """Convert a MongoDB-compatible type to a Python type.""" - if not self.dbref and not isinstance( - value, (DBRef, Document, EmbeddedDocument) - ): + if not isinstance(value, (DBRef, Document, EmbeddedDocument)): collection = self.document_type._get_collection_name() value = DBRef(collection, self.document_type.id.to_python(value)) return value From cfd4d6a161556ef4a8aa355468384554eb684442 Mon Sep 17 00:00:00 2001 From: Eric Timmons Date: Sun, 15 Dec 2019 12:02:24 -0500 Subject: [PATCH 6/6] Add breaking change to changelog for LazyReferenceField representation in _data --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index bc01a403..b308c5fb 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,7 @@ Development - If you catch/use ``MongoEngineConnectionError`` in your code, you'll have to rename it. - BREAKING CHANGE: Positional arguments when instantiating a document are no longer supported. #2103 - From now on keyword arguments (e.g. ``Doc(field_name=value)``) are required. +- BREAKING CHANGE: A ``LazyReferenceField`` is now stored in the ``_data`` field of its parent as a ``DBRef``, ``Document``, or ``EmbeddedDocument`` (``ObjectId`` is no longer allowed). #2182 - DEPRECATION: ``Q.empty`` & ``QNode.empty`` are marked as deprecated and will be removed in a next version of MongoEngine. #2210 - Added ability to check if Q or QNode are empty by parsing them to bool. - Instead of ``Q(name="John").empty`` use ``not Q(name="John")``.