Merge pull request #2723 from bagerard/clone_null_check_oid

Work on top of "Added null check for Issue"

Author: bagerard

docs/changelog.rst

@@ -19,6 +19,7 @@ Development
   Although it could work to switch an existing DecimalField to Decimal128Field without applying a migration script,
   it is not recommended to do so (DecimalField uses float/str to store the value, Decimal128Field uses Decimal128).
 - BREAKING CHANGE: When using ListField(EnumField) or DictField(EnumField), the values weren't always cast into the Enum (#2531)
+- BREAKING CHANGE (bugfix) Querying ObjectIdField or ComplexDateTimeField with None will no longer raise a ValidationError (#2681)
 
 Changes in 0.25.0
 =================

mongoengine/base/document.py

@@ -372,7 +372,7 @@ def to_mongo(self, use_db_field=True, fields=None):
                 value = field.generate()
                 self._data[field_name] = value
 
-            if (value is not None) or (field.null):
+            if value is not None or field.null:
                 if use_db_field:
                     data[field.db_field] = value
                 else:

mongoengine/base/fields.py

@@ -52,20 +52,20 @@ def __init__(
         :param required: If the field is required. Whether it has to have a
             value or not. Defaults to False.
         :param default: (optional) The default value for this field if no value
-            has been set (or if the value has been unset).  It can be a
+            has been set, if the value is set to None or has been unset. It can be a
             callable.
-        :param unique: Is the field value unique or not.  Defaults to False.
+        :param unique: Is the field value unique or not (Creates an index).  Defaults to False.
         :param unique_with: (optional) The other field this field should be
-            unique with.
-        :param primary_key: Mark this field as the primary key. Defaults to False.
+            unique with (Creates an index).
+        :param primary_key: Mark this field as the primary key ((Creates an index)). Defaults to False.
         :param validation: (optional) A callable to validate the value of the
             field.  The callable takes the value as parameter and should raise
             a ValidationError if validation fails
         :param choices: (optional) The valid choices
-        :param null: (optional) If the field value can be null. If no and there is a default value
-            then the default value is set
+        :param null: (optional) If the field value can be null when a default exist. If not set, the default value
+        will be used in case a field with a default value is set to None. Defaults to False.
         :param sparse: (optional) `sparse=True` combined with `unique=True` and `required=False`
-            means that uniqueness won't be enforced for `None` values
+            means that uniqueness won't be enforced for `None` values (Creates an index). Defaults to False.
         :param **kwargs: (optional) Arbitrary indirection-free metadata for
             this field can be supplied as additional keyword arguments and
             accessed as attributes of the field. Must not conflict with any
@@ -521,14 +521,17 @@ def to_python(self, value):
         return value
 
     def to_mongo(self, value):
-        if not isinstance(value, ObjectId):
-            try:
-                return ObjectId(str(value))
-            except Exception as e:
-                self.error(str(e))
-        return value
+        if isinstance(value, ObjectId):
+            return value
+
+        try:
+            return ObjectId(str(value))
+        except Exception as e:
+            self.error(str(e))
 
     def prepare_query_value(self, op, value):
+        if value is None:
+            return value
         return self.to_mongo(value)
 
     def validate(self, value):

mongoengine/document.py

@@ -422,7 +422,9 @@ def save(
         try:
             # Save a new document or update an existing one
             if created:
-                object_id = self._save_create(doc, force_insert, write_concern)
+                object_id = self._save_create(
+                    doc=doc, force_insert=force_insert, write_concern=write_concern
+                )
             else:
                 object_id, created = self._save_update(
                     doc, save_condition, write_concern

mongoengine/fields.py

@@ -490,9 +490,6 @@ def __init__(
         super().__init__(**kwargs)
 
     def to_python(self, value):
-        if value is None:
-            return value
-
         # Convert to string for python 2.6 before casting to Decimal
         try:
             value = decimal.Decimal("%s" % value)
@@ -506,8 +503,6 @@ def to_python(self, value):
             return value.quantize(decimal.Decimal(), rounding=self.rounding)
 
     def to_mongo(self, value):
-        if value is None:
-            return value
         if self.force_string:
             return str(self.to_python(value))
         return float(self.to_python(value))
@@ -528,6 +523,8 @@ def validate(self, value):
             self.error("Decimal value is too large")
 
     def prepare_query_value(self, op, value):
+        if value is None:
+            return value
         return super().prepare_query_value(op, self.to_mongo(value))
 
 
@@ -731,6 +728,8 @@ def to_mongo(self, value):
         return self._convert_from_datetime(value)
 
     def prepare_query_value(self, op, value):
+        if value is None:
+            return value
         return super().prepare_query_value(op, self._convert_from_datetime(value))
 
 

tests/document/test_instance.py

@@ -4,6 +4,7 @@
 import uuid
 import weakref
 from datetime import datetime
+from unittest.mock import Mock
 
 import bson
 import pytest
@@ -1034,6 +1035,17 @@ def test_save(self):
             "_id": person.id,
         }
 
+    def test_save_write_concern(self):
+        class Recipient(Document):
+            email = EmailField(required=True)
+
+        rec = Recipient(email="garbage@garbage.com")
+
+        fn = Mock()
+        rec._save_create = fn
+        rec.save(write_concern={"w": 0})
+        assert fn.call_args[1]["write_concern"] == {"w": 0}
+
     def test_save_skip_validation(self):
         class Recipient(Document):
             email = EmailField(required=True)
@@ -3593,8 +3605,7 @@ class User(Document):
             cdt_fld = ComplexDateTimeField(null=True)
 
         User.objects.delete()
-        u = User(name="user")
-        u.save()
+        u = User(name="user").save()
         u_from_db = User.objects.get(name="user")
         u_from_db.height = None
         u_from_db.save()

tests/fields/test_complex_datetime_field.py

@@ -206,3 +206,9 @@ class Log(Document):
 
         with pytest.raises(ValidationError):
             log.save()
+
+    def test_query_none_value_dont_raise(self):
+        class Log(Document):
+            timestamp = ComplexDateTimeField()
+
+        _ = list(Log.objects(timestamp=None))

tests/fields/test_decimal_field.py

@@ -71,6 +71,8 @@ class Person(Document):
         fetched_person = Person.objects.first()
         fetched_person.value is None
 
+        assert Person.objects(value=None).first() is not None
+
     def test_validation(self):
         """Ensure that invalid values cannot be assigned to decimal fields."""
 

tests/fields/test_float_field.py

@@ -57,3 +57,9 @@ class BigPerson(Document):
         big_person.height = 2**100000  # Too big for a float value
         with pytest.raises(ValidationError):
             big_person.validate()
+
+    def test_query_none_value_dont_raise(self):
+        class BigPerson(Document):
+            height = FloatField()
+
+        _ = list(BigPerson.objects(height=None))

tests/fields/test_object_id_field.py

@@ -28,3 +28,10 @@ class MyDoc(Document):
         doc = MyDoc(oid="not-an-oid!")
         with pytest.raises(ValidationError, match="Invalid ObjectID"):
             doc.save()
+
+    def test_query_none_value_dont_raise(self):
+        # cf issue #2681
+        class MyDoc(Document):
+            oid = ObjectIdField(null=True)
+
+        _ = list(MyDoc.objects(oid=None))