Fixed amibiguity and differing behaviour regarding field defaults (#349)

Now field defaults are king, unsetting or setting to None on a field
with a default means the default is reapplied.

AUTHORS

@@ -167,3 +167,4 @@ that much better:
  * zhy0216 (https://github.com/zhy0216)
  * istinspring (https://github.com/istinspring)
  * Massimo Santini (https://github.com/mapio)
+ * Nigel McNie (https://github.com/nigelmcnie)

docs/apireference.rst

@@ -54,6 +54,7 @@ Querying
 Fields
 ======
 
+.. autoclass:: mongoengine.base.fields.BaseField
 .. autoclass:: mongoengine.fields.StringField
 .. autoclass:: mongoengine.fields.URLField
 .. autoclass:: mongoengine.fields.EmailField

docs/changelog.rst

@@ -5,6 +5,7 @@ Changelog
 
 Changes in 0.8.2
 ================
+- Fixed amibiguity and differing behaviour regarding field defaults (#349)
 - ImageFields now include PIL error messages if invalid error (#353)
 - Added lock when calling doc.Delete() for when signals have no sender (#350)
 - Reload forces read preference to be PRIMARY (#355)

mongoengine/base/fields.py

@@ -36,6 +36,29 @@ class BaseField(object):
                  unique=False, unique_with=None, primary_key=False,
                  validation=None, choices=None, verbose_name=None,
                  help_text=None):
+        """
+        :param db_field: The database field to store this field in
+            (defaults to the name of the field)
+        :param name: Depreciated - use db_field
+        :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
+            callable.
+        :param unique: Is the field value unique or not.  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.
+        :param validation: (optional) A callable to validate the value of the
+            field.  Generally this is deprecated in favour of the
+            `FIELD.validate` method
+        :param choices: (optional) The valid choices
+        :param verbose_name: (optional)  The verbose name for the field.
+            Designed to be human readable and is often used when generating
+            model forms from the document model.
+        :param help_text: (optional) The help text for this field and is often
+            used when generating model forms from the document model.
+        """
         self.db_field = (db_field or name) if not primary_key else '_id'
         if name:
             msg = "Fields' 'name' attribute deprecated in favour of 'db_field'"
@@ -65,14 +88,9 @@ class BaseField(object):
         if instance is None:
             # Document class being used rather than a document object
             return self
-        # Get value from document instance if available, if not use default
-        value = instance._data.get(self.name)
 
-        if value is None:
+        # Get value from document instance if available
-            value = self.default
+        value = instance._data.get(self.name)
-            # Allow callable default values
-            if callable(value):
-                value = value()
 
         EmbeddedDocument = _import_class('EmbeddedDocument')
         if isinstance(value, EmbeddedDocument) and value._instance is None:
@@ -82,9 +100,11 @@ class BaseField(object):
     def __set__(self, instance, value):
         """Descriptor for assigning a value to a field in a document.
         """
-        if value is None:
+
+        # If setting to None and theres a default
+        # Then set the value to the default value
+        if value is None and self.default is not None:
             value = self.default
-            # Allow callable default values
             if callable(value):
                 value = value()
 

mongoengine/document.py

@@ -206,7 +206,7 @@ class Document(BaseDocument):
         if validate:
             self.validate(clean=clean)
 
-        if not write_concern:
+        if write_concern is None:
             write_concern = {"w": 1}
 
         doc = self.to_mongo()

mongoengine/queryset/queryset.py

@@ -348,7 +348,7 @@ class QuerySet(object):
         """
         Document = _import_class('Document')
 
-        if not write_concern:
+        if write_concern is None:
             write_concern = {}
 
         docs = doc_or_docs
@@ -424,7 +424,7 @@ class QuerySet(object):
         queryset = self.clone()
         doc = queryset._document
 
-        if not write_concern:
+        if write_concern is None:
             write_concern = {}
 
         # Handle deletes where skips or limits have been applied or
@@ -490,7 +490,7 @@ class QuerySet(object):
         if not update and not upsert:
             raise OperationError("No update parameters, would remove data")
 
-        if not write_concern:
+        if write_concern is None:
             write_concern = {}
 
         queryset = self.clone()

tests/fields/fields.py

@@ -34,33 +34,137 @@ class FieldTest(unittest.TestCase):
         self.db.drop_collection('fs.files')
         self.db.drop_collection('fs.chunks')
 
-    def test_default_values(self):
+    def test_default_values_nothing_set(self):
         """Ensure that default field values are used when creating a document.
         """
         class Person(Document):
             name = StringField()
-            age = IntField(default=30, help_text="Your real age")
+            age = IntField(default=30, required=False)
-            userid = StringField(default=lambda: 'test', verbose_name="User Identity")
+            userid = StringField(default=lambda: 'test', required=True)
-
-        person = Person(name='Test Person')
-        self.assertEqual(person._data['age'], 30)
-        self.assertEqual(person._data['userid'], 'test')
-        self.assertEqual(person._fields['name'].help_text, None)
-        self.assertEqual(person._fields['age'].help_text, "Your real age")
-        self.assertEqual(person._fields['userid'].verbose_name, "User Identity")
-
-        class Person2(Document):
             created = DateTimeField(default=datetime.datetime.utcnow)
 
-        person = Person2()
+        person = Person(name="Ross")
-        date1 = person.created
-        date2 = person.created
-        self.assertEqual(date1, date2)
 
-        person = Person2(created=None)
+        # Confirm saving now would store values
-        date1 = person.created
+        data_to_be_saved = sorted(person.to_mongo().keys())
-        date2 = person.created
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'name', 'userid'])
-        self.assertEqual(date1, date2)
+
+        self.assertTrue(person.validate() is None)
+
+        self.assertEqual(person.name, person.name)
+        self.assertEqual(person.age, person.age)
+        self.assertEqual(person.userid, person.userid)
+        self.assertEqual(person.created, person.created)
+
+        self.assertEqual(person._data['name'], person.name)
+        self.assertEqual(person._data['age'], person.age)
+        self.assertEqual(person._data['userid'], person.userid)
+        self.assertEqual(person._data['created'], person.created)
+
+        # Confirm introspection changes nothing
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'name', 'userid'])
+
+    def test_default_values_set_to_None(self):
+        """Ensure that default field values are used when creating a document.
+        """
+        class Person(Document):
+            name = StringField()
+            age = IntField(default=30, required=False)
+            userid = StringField(default=lambda: 'test', required=True)
+            created = DateTimeField(default=datetime.datetime.utcnow)
+
+        # Trying setting values to None
+        person = Person(name=None, age=None, userid=None, created=None)
+
+        # Confirm saving now would store values
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
+
+        self.assertTrue(person.validate() is None)
+
+        self.assertEqual(person.name, person.name)
+        self.assertEqual(person.age, person.age)
+        self.assertEqual(person.userid, person.userid)
+        self.assertEqual(person.created, person.created)
+
+        self.assertEqual(person._data['name'], person.name)
+        self.assertEqual(person._data['age'], person.age)
+        self.assertEqual(person._data['userid'], person.userid)
+        self.assertEqual(person._data['created'], person.created)
+
+        # Confirm introspection changes nothing
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
+
+    def test_default_values_when_setting_to_None(self):
+        """Ensure that default field values are used when creating a document.
+        """
+        class Person(Document):
+            name = StringField()
+            age = IntField(default=30, required=False)
+            userid = StringField(default=lambda: 'test', required=True)
+            created = DateTimeField(default=datetime.datetime.utcnow)
+
+        person = Person()
+        person.name = None
+        person.age = None
+        person.userid = None
+        person.created = None
+
+        # Confirm saving now would store values
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
+
+        self.assertTrue(person.validate() is None)
+
+        self.assertEqual(person.name, person.name)
+        self.assertEqual(person.age, person.age)
+        self.assertEqual(person.userid, person.userid)
+        self.assertEqual(person.created, person.created)
+
+        self.assertEqual(person._data['name'], person.name)
+        self.assertEqual(person._data['age'], person.age)
+        self.assertEqual(person._data['userid'], person.userid)
+        self.assertEqual(person._data['created'], person.created)
+
+        # Confirm introspection changes nothing
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
+
+    def test_default_values_when_deleting_value(self):
+        """Ensure that default field values are used when creating a document.
+        """
+        class Person(Document):
+            name = StringField()
+            age = IntField(default=30, required=False)
+            userid = StringField(default=lambda: 'test', required=True)
+            created = DateTimeField(default=datetime.datetime.utcnow)
+
+        person = Person(name="Ross")
+        del person.name
+        del person.age
+        del person.userid
+        del person.created
+
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
+
+        self.assertTrue(person.validate() is None)
+
+        self.assertEqual(person.name, person.name)
+        self.assertEqual(person.age, person.age)
+        self.assertEqual(person.userid, person.userid)
+        self.assertEqual(person.created, person.created)
+
+        self.assertEqual(person._data['name'], person.name)
+        self.assertEqual(person._data['age'], person.age)
+        self.assertEqual(person._data['userid'], person.userid)
+        self.assertEqual(person._data['created'], person.created)
+
+        # Confirm introspection changes nothing
+        data_to_be_saved = sorted(person.to_mongo().keys())
+        self.assertEqual(data_to_be_saved, ['age', 'created', 'userid'])
 
     def test_required_values(self):
         """Ensure that required field constraints are enforced.