From f83ae5789b26b09192b4bdbefedd5cfffa6cc04c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastien=20G=C3=A9rard?= Date: Sun, 16 Sep 2018 21:27:09 +0200 Subject: [PATCH] fix side effect from queryset's no_dereference #1677 --- mongoengine/base/document.py | 2 +- mongoengine/base/fields.py | 8 ++-- mongoengine/fields.py | 5 ++- tests/queryset/queryset.py | 84 +++++++++++++++++++++++++++++++----- 4 files changed, 83 insertions(+), 16 deletions(-) diff --git a/mongoengine/base/document.py b/mongoengine/base/document.py index 84acb5a2..c82d670e 100644 --- a/mongoengine/base/document.py +++ b/mongoengine/base/document.py @@ -700,7 +700,7 @@ class BaseDocument(object): fields = cls._fields if not _auto_dereference: - fields = copy.copy(fields) + fields = copy.deepcopy(fields) for field_name, field in fields.iteritems(): field._auto_dereference = _auto_dereference diff --git a/mongoengine/base/fields.py b/mongoengine/base/fields.py index a0726aa6..6c1bf4f1 100644 --- a/mongoengine/base/fields.py +++ b/mongoengine/base/fields.py @@ -266,13 +266,15 @@ class ComplexBaseField(BaseField): ReferenceField = _import_class('ReferenceField') GenericReferenceField = _import_class('GenericReferenceField') EmbeddedDocumentListField = _import_class('EmbeddedDocumentListField') - dereference = (self._auto_dereference and + + auto_dereference = instance._fields[self.name]._auto_dereference + + dereference = (auto_dereference and (self.field is None or isinstance(self.field, (GenericReferenceField, ReferenceField)))) _dereference = _import_class('DeReference')() - self._auto_dereference = instance._fields[self.name]._auto_dereference if instance._initialised and dereference and instance._data.get(self.name): instance._data[self.name] = _dereference( instance._data.get(self.name), max_depth=1, instance=instance, @@ -293,7 +295,7 @@ class ComplexBaseField(BaseField): value = BaseDict(value, instance, self.name) instance._data[self.name] = value - if (self._auto_dereference and instance._initialised and + if (auto_dereference and instance._initialised and isinstance(value, (BaseList, BaseDict)) and not value._dereferenced): value = _dereference( diff --git a/mongoengine/fields.py b/mongoengine/fields.py index f794edff..731ab06b 100644 --- a/mongoengine/fields.py +++ b/mongoengine/fields.py @@ -1104,9 +1104,9 @@ class ReferenceField(BaseField): # Get value from document instance if available value = instance._data.get(self.name) - self._auto_dereference = instance._fields[self.name]._auto_dereference + auto_dereference = instance._fields[self.name]._auto_dereference # Dereference DBRefs - if self._auto_dereference and isinstance(value, DBRef): + if auto_dereference and isinstance(value, DBRef): if hasattr(value, 'cls'): # Dereference using the class type specified in the reference cls = get_document(value.cls) @@ -1267,6 +1267,7 @@ class CachedReferenceField(BaseField): # Get value from document instance if available value = instance._data.get(self.name) self._auto_dereference = instance._fields[self.name]._auto_dereference + # Dereference DBRefs if self._auto_dereference and isinstance(value, DBRef): dereferenced = self.document_type._get_db().dereference(value) diff --git a/tests/queryset/queryset.py b/tests/queryset/queryset.py index 0f2364f7..35ebe24d 100644 --- a/tests/queryset/queryset.py +++ b/tests/queryset/queryset.py @@ -4674,11 +4674,69 @@ class QuerySetTest(unittest.TestCase): User(name="Bob Dole", organization=whitehouse).save() qs = User.objects() + qs_user = qs.first() + self.assertIsInstance(qs.first().organization, Organization) - self.assertNotIsInstance(qs.no_dereference().first().organization, Organization) - self.assertNotIsInstance(qs.no_dereference().get().organization, Organization) + + self.assertIsInstance(qs.no_dereference().first().organization, DBRef) + + self.assertIsInstance(qs_user.organization, Organization) self.assertIsInstance(qs.first().organization, Organization) + def test_no_dereference_internals(self): + # Test the internals on which queryset.no_dereference relies on + class Organization(Document): + name = StringField() + + class User(Document): + organization = ReferenceField(Organization) + + User.drop_collection() + Organization.drop_collection() + + cls_organization_field = User.organization + self.assertTrue(cls_organization_field._auto_dereference, True) # default + + org = Organization(name="whatever").save() + User(organization=org).save() + + qs_no_deref = User.objects().no_dereference() + user_no_deref = qs_no_deref.first() + self.assertFalse(qs_no_deref._auto_dereference) + + # Make sure the instance field is different from the class field + instance_org_field = user_no_deref._fields['organization'] + self.assertIsNot(instance_org_field, cls_organization_field) + self.assertFalse(instance_org_field._auto_dereference) + + self.assertIsInstance(user_no_deref.organization, DBRef) + self.assertTrue(cls_organization_field._auto_dereference, True) # Make sure the class Field wasn't altered + + def test_no_dereference_no_side_effect_on_existing_instance(self): + # Relates to issue #1677 - ensures no regression of the bug + + class Organization(Document): + name = StringField() + + class User(Document): + organization = ReferenceField(Organization) + + User.drop_collection() + Organization.drop_collection() + + org = Organization(name="whatever").save() + User(organization=org).save() + + qs = User.objects() + user = qs.first() + + qs_no_deref = User.objects().no_dereference() + user_no_deref = qs_no_deref.first() + + no_derf_org = user_no_deref.organization # was triggering the bug + self.assertIsInstance(no_derf_org, DBRef) + self.assertIsInstance(user.organization, Organization) + def test_no_dereference_embedded_doc(self): class User(Document): @@ -4693,7 +4751,7 @@ class QuerySetTest(unittest.TestCase): members = ListField(EmbeddedDocumentField(Member)) ceo = ReferenceField(User) member = EmbeddedDocumentField(Member) - admin = ListField(ReferenceField(User)) + admins = ListField(ReferenceField(User)) Organization.drop_collection() User.drop_collection() @@ -4703,16 +4761,22 @@ class QuerySetTest(unittest.TestCase): member = Member(name="Flash", user=user) - company = Organization(name="Mongo Inc", ceo=user, member=member) - company.admin.append(user) - company.members.append(member) + company = Organization(name="Mongo Inc", + ceo=user, + member=member, + admins=[user], + members=[member]) company.save() - result = Organization.objects().no_dereference().first() + org = Organization.objects().no_dereference().first() - self.assertIsInstance(result.admin[0], (DBRef, ObjectId)) - self.assertIsInstance(result.member.user, (DBRef, ObjectId)) - self.assertIsInstance(result.members[0].user, (DBRef, ObjectId)) + self.assertNotEqual(id(org._fields['admins']), id(Organization.admins)) + self.assertFalse(org._fields['admins']._auto_dereference) + + admin = org.admins[0] + self.assertIsInstance(admin, DBRef) + self.assertIsInstance(org.member.user, DBRef) + self.assertIsInstance(org.members[0].user, DBRef) def test_cached_queryset(self): class Person(Document):