From 4a44eeb1c90982faa5f842fa9fe4d2431eeeaaec Mon Sep 17 00:00:00 2001 From: Bastien Gerard Date: Tue, 8 Oct 2024 22:54:24 +0200 Subject: [PATCH] Improve error message in case a document assigned to a ReferenceField wasn't saved yet --- docs/changelog.rst | 2 +- mongoengine/fields.py | 53 ++++++++++------------------ tests/fields/test_reference_field.py | 14 +++++--- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 887a67773..014a83991 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,7 +22,7 @@ Development - Add support for collation/hint/comment to delete/update and aggregate #2842 - BREAKING CHANGE: Remove LongField as it's equivalent to IntField since we drop support to Python2 long time ago (User should simply switch to IntField) #2309 - BugFix - Calling .clear on a ListField wasn't being marked as changed (and flushed to db upon .save()) #2858 - +- Improve error message in case a document assigned to a ReferenceField wasn't saved yet #1955 Changes in 0.29.0 ================= diff --git a/mongoengine/fields.py b/mongoengine/fields.py index e9cf5b817..980098dfb 100644 --- a/mongoengine/fields.py +++ b/mongoengine/fields.py @@ -111,6 +111,14 @@ RECURSIVE_REFERENCE_CONSTANT = "self" +def _unsaved_object_error(document): + return ( + f"The instance of the document '{document}' you are " + "trying to reference has an empty 'id'. You can only reference " + "documents once they have been saved to the database" + ) + + class StringField(BaseField): """A unicode string field.""" @@ -1215,10 +1223,7 @@ def to_mongo(self, document): # XXX ValidationError raised outside of the "validate" method. if id_ is None: - self.error( - "You can only reference documents once they have" - " been saved to the database" - ) + self.error(_unsaved_object_error(document.__class__.__name__)) # Use the attributes from the document instance, so that they # override the attributes of this field's document type @@ -1262,10 +1267,7 @@ def validate(self, value): ) if isinstance(value, Document) and value.id is None: - self.error( - "You can only reference documents once they have been " - "saved to the database" - ) + self.error(_unsaved_object_error(value.__class__.__name__)) def lookup_member(self, member_name): return self.document_type._fields.get(member_name) @@ -1370,10 +1372,7 @@ def to_mongo(self, document, use_db_field=True, fields=None): # We need the id from the saved object to create the DBRef id_ = document.pk if id_ is None: - self.error( - "You can only reference documents once they have" - " been saved to the database" - ) + self.error(_unsaved_object_error(document.__class__.__name__)) else: self.error("Only accept a document object") @@ -1394,10 +1393,7 @@ def prepare_query_value(self, op, value): # XXX ValidationError raised outside of the "validate" method. if isinstance(value, Document): if value.pk is None: - self.error( - "You can only reference documents once they have" - " been saved to the database" - ) + self.error(_unsaved_object_error(value.__class__.__name__)) value_dict = {"_id": value.pk} for field in self.fields: value_dict.update({field: value[field]}) @@ -1411,10 +1407,7 @@ def validate(self, value): self.error("A CachedReferenceField only accepts documents") if isinstance(value, Document) and value.id is None: - self.error( - "You can only reference documents once they have been " - "saved to the database" - ) + self.error(_unsaved_object_error(value.__class__.__name__)) def lookup_member(self, member_name): return self.document_type._fields.get(member_name) @@ -1513,10 +1506,7 @@ def validate(self, value): # We need the id from the saved object to create the DBRef elif isinstance(value, Document) and value.id is None: - self.error( - "You can only reference documents once they have been" - " saved to the database" - ) + self.error(_unsaved_object_error(value.__class__.__name__)) def to_mongo(self, document): if document is None: @@ -1533,10 +1523,7 @@ def to_mongo(self, document): id_ = document.id if id_ is None: # XXX ValidationError raised outside of the "validate" method. - self.error( - "You can only reference documents once they have" - " been saved to the database" - ) + self.error(_unsaved_object_error(document.__class__.__name__)) else: id_ = document @@ -2535,10 +2522,7 @@ def validate(self, value): ) if pk is None: - self.error( - "You can only reference documents once they have been " - "saved to the database" - ) + self.error(_unsaved_object_error(self.document_type.__name__)) def prepare_query_value(self, op, value): if value is None: @@ -2607,8 +2591,9 @@ def __get__(self, instance, owner): def validate(self, value): if isinstance(value, LazyReference) and value.pk is None: self.error( - "You can only reference documents once they have been" - " saved to the database" + _unsaved_object_error( + self.__class__.__name__ + ) # Actual class is difficult to predict here ) return super().validate(value) diff --git a/tests/fields/test_reference_field.py b/tests/fields/test_reference_field.py index 94869f2ea..55ffb6845 100644 --- a/tests/fields/test_reference_field.py +++ b/tests/fields/test_reference_field.py @@ -20,7 +20,7 @@ class Test(Document): class NonDocumentSubClass: pass - # fails if given an non Document subclass + # fails if given a non Document subclass with pytest.raises(ValidationError, match=ERROR_MSG): class Test(Document): # noqa: F811 @@ -46,12 +46,17 @@ class BlogPost(Document): with pytest.raises(ValidationError): ReferenceField(EmbeddedDocument) - user = User(name="Test User") + unsaved_user = User(name="Test User") # Ensure that the referenced object must have been saved post1 = BlogPost(content="Chips and gravy taste good.") - post1.author = user - with pytest.raises(ValidationError): + post1.author = unsaved_user + expected_error = ( + "The instance of the document 'User' you are " + "trying to reference has an empty 'id'. You can only reference " + "documents once they have been saved to the database" + ) + with pytest.raises(ValidationError, match=expected_error): post1.save() # Check that an invalid object type cannot be used @@ -61,6 +66,7 @@ class BlogPost(Document): post1.validate() # Ensure ObjectID's are accepted as references + user = User(name="Test User") user_object_id = user.pk post3 = BlogPost(content="Chips and curry sauce taste good.") post3.author = user_object_id