Skip to content

Commit

Permalink
Improve error message in case a document assigned to a ReferenceField…
Browse files Browse the repository at this point in the history
… wasn't saved yet
  • Loading branch information
bagerard committed Oct 8, 2024
1 parent e77daa6 commit 4a44eeb
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 39 deletions.
2 changes: 1 addition & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=================
Expand Down
53 changes: 19 additions & 34 deletions mongoengine/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")

Expand All @@ -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]})
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
14 changes: 10 additions & 4 deletions tests/fields/test_reference_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 4a44eeb

Please sign in to comment.