better db_field validation

Externalize Sphinx RTD theme

.travis.yml

@@ -16,7 +16,8 @@ python:
 - 2.7
 - 3.5
 - pypy
-- pypy3
+- pypy3.3-5.2-alpha1
+
 
 env:
 - MONGODB=2.6 PYMONGO=2.7

mongoengine/base/fields.py

@@ -81,7 +81,14 @@ class BaseField(object):
         self.sparse = sparse
         self._owner_document = None
 
-        # Validate the db_field
+        # Make sure db_field is a string (if it's explicitly defined).
+        if (
+            self.db_field is not None and
+            not isinstance(self.db_field, six.string_types)
+        ):
+            raise TypeError('db_field should be a string.')
+
+        # Make sure db_field doesn't contain any forbidden characters.
         if isinstance(self.db_field, six.string_types) and (
             '.' in self.db_field or
             '\0' in self.db_field or

mongoengine/document.py

@@ -300,7 +300,7 @@ class Document(BaseDocument):
         created.
 
         :param force_insert: only try to create a new document, don't allow
-            updates of existing documents
+            updates of existing documents.
         :param validate: validates the document; set to ``False`` to skip.
         :param clean: call the document clean method, requires `validate` to be
             True.
@@ -441,6 +441,21 @@ class Document(BaseDocument):
 
         return object_id
 
+    def _get_update_doc(self):
+        """Return a dict containing all the $set and $unset operations
+        that should be sent to MongoDB based on the changes made to this
+        Document.
+        """
+        updates, removals = self._delta()
+
+        update_doc = {}
+        if updates:
+            update_doc['$set'] = updates
+        if removals:
+            update_doc['$unset'] = removals
+
+        return update_doc
+
     def _save_update(self, doc, save_condition, write_concern):
         """Update an existing document.
 
@@ -466,15 +481,10 @@ class Document(BaseDocument):
                 val = val[ak]
             select_dict['.'.join(actual_key)] = val
 
-        updates, removals = self._delta()
-        update_query = {}
-        if updates:
-            update_query['$set'] = updates
-        if removals:
-            update_query['$unset'] = removals
-        if updates or removals:
+        update_doc = self._get_update_doc()
+        if update_doc:
             upsert = save_condition is None
-            last_error = collection.update(select_dict, update_query,
+            last_error = collection.update(select_dict, update_doc,
                                            upsert=upsert, **write_concern)
             if not upsert and last_error['n'] == 0:
                 raise SaveConditionError('Race condition preventing'

tests/document/instance.py

@@ -242,7 +242,7 @@ class InstanceTest(unittest.TestCase):
         Zoo.drop_collection()
 
         class Zoo(Document):
-            animals = ListField(GenericReferenceField(Animal))
+            animals = ListField(GenericReferenceField())
 
         # Save a reference to each animal
         zoo = Zoo(animals=Animal.objects)

tests/queryset/queryset.py

@@ -917,7 +917,9 @@ class QuerySetTest(unittest.TestCase):
         self.assertEqual(Blog.objects.count(), 3)
 
     def test_get_changed_fields_query_count(self):
-
+        """Make sure we don't perform unnecessary db operations when
+        none of document's fields were updated.
+        """
         class Person(Document):
             name = StringField()
             owns = ListField(ReferenceField('Organization'))
@@ -925,8 +927,8 @@ class QuerySetTest(unittest.TestCase):
 
         class Organization(Document):
             name = StringField()
-            owner = ReferenceField('Person')
-            employees = ListField(ReferenceField('Person'))
+            owner = ReferenceField(Person)
+            employees = ListField(ReferenceField(Person))
 
         class Project(Document):
             name = StringField()
@@ -945,35 +947,35 @@ class QuerySetTest(unittest.TestCase):
         with query_counter() as q:
             self.assertEqual(q, 0)
 
-            fresh_o1 = Organization.objects.get(id=o1.id)
-            self.assertEqual(1, q)
-            fresh_o1._get_changed_fields()
-            self.assertEqual(1, q)
-
-        with query_counter() as q:
-            self.assertEqual(q, 0)
-
-            fresh_o1 = Organization.objects.get(id=o1.id)
-            fresh_o1.save()   # No changes, does nothing
-
+            # Fetching a document should result in a query.
+            org = Organization.objects.get(id=o1.id)
             self.assertEqual(q, 1)
 
-        with query_counter() as q:
-            self.assertEqual(q, 0)
-
-            fresh_o1 = Organization.objects.get(id=o1.id)
-            fresh_o1.save(cascade=False)  # No changes, does nothing
-
+            # Checking changed fields of a newly fetched document should not
+            # result in a query.
+            org._get_changed_fields()
             self.assertEqual(q, 1)
 
+        # Saving a doc without changing any of its fields should not result
+        # in a query (with or without cascade=False).
+        org = Organization.objects.get(id=o1.id)
         with query_counter() as q:
+            org.save()
             self.assertEqual(q, 0)
 
-            fresh_o1 = Organization.objects.get(id=o1.id)
-            fresh_o1.employees.append(p2)  # Dereferences
-            fresh_o1.save(cascade=False)   # Saves
+        org = Organization.objects.get(id=o1.id)
+        with query_counter() as q:
+            org.save(cascade=False)
+            self.assertEqual(q, 0)
 
-            self.assertEqual(q, 3)
+        # Saving a doc after you append a reference to it should result in
+        # two db operations (a query for the reference and an update).
+        # TODO dereferencing of p2 shouldn't be necessary.
+        org = Organization.objects.get(id=o1.id)
+        with query_counter() as q:
+            org.employees.append(p2)  # dereferences p2
+            org.save()  # saves the org
+            self.assertEqual(q, 2)
 
     @skip_pymongo3
     def test_slave_okay(self):