make EmbeddedDocument not hashable by default

docs/tutorial.rst

@@ -206,10 +206,7 @@ object::
         ross.last_name = 'Lawley'
         ross.save()
 
-Assign another user to a variable called ``john``, just like we did above with
+Now that we've got our user in the database, let's add a couple of posts::
-``ross``.
-
-Now that we've got our users in the database, let's add a couple of posts::
 
     post1 = TextPost(title='Fun with MongoEngine', author=john)
     post1.content = 'Took a look at MongoEngine today, looks pretty cool.'

mongoengine/base/document.py

@@ -272,13 +272,6 @@ class BaseDocument(object):
     def __ne__(self, other):
         return not self.__eq__(other)
 
-    def __hash__(self):
-        if getattr(self, 'pk', None) is None:
-            # For new object
-            return super(BaseDocument, self).__hash__()
-        else:
-            return hash(self.pk)
-
     def clean(self):
         """
         Hook for doing document level data cleaning before validation is run.

mongoengine/document.py

@@ -60,6 +60,12 @@ class EmbeddedDocument(BaseDocument):
     my_metaclass = DocumentMetaclass
     __metaclass__ = DocumentMetaclass
 
+    # A generic embedded document doesn't have any immutable properties
+    # that describe it uniquely, hence it shouldn't be hashable. You can
+    # define your own __hash__ method on a subclass if you need your
+    # embedded documents to be hashable.
+    __hash__ = None
+
     def __init__(self, *args, **kwargs):
         super(EmbeddedDocument, self).__init__(*args, **kwargs)
         self._instance = None
@@ -160,6 +166,15 @@ class Document(BaseDocument):
         """Set the primary key."""
         return setattr(self, self._meta['id_field'], value)
 
+    def __hash__(self):
+        """Return the hash based on the PK of this document. If it's new
+        and doesn't have a PK yet, return the default object hash instead.
+        """
+        if self.pk is None:
+            return super(BaseDocument, self).__hash__()
+        else:
+            return hash(self.pk)
+
     @classmethod
     def _get_db(cls):
         """Some Model using other db_alias"""
@@ -167,63 +182,45 @@ class Document(BaseDocument):
 
     @classmethod
     def _get_collection(cls):
-        """Return a PyMongo collection for the document."""
+        """Returns the collection for the document."""
+        # TODO: use new get_collection() with PyMongo3 ?
         if not hasattr(cls, '_collection') or cls._collection is None:
-
+            db = cls._get_db()
-            # Get the collection, either capped or regular.
+            collection_name = cls._get_collection_name()
+            # Create collection as a capped collection if specified
             if cls._meta.get('max_size') or cls._meta.get('max_documents'):
-                cls._collection = cls._get_capped_collection()
+                # Get max document limit and max byte size from meta
-            else:
+                max_size = cls._meta.get('max_size') or 10 * 2 ** 20  # 10MB default
-                db = cls._get_db()
+                max_documents = cls._meta.get('max_documents')
-                collection_name = cls._get_collection_name()
+                # Round up to next 256 bytes as MongoDB would do it to avoid exception
-                cls._collection = db[collection_name]
+                if max_size % 256:
+                    max_size = (max_size // 256 + 1) * 256
 
-            # Ensure indexes on the collection unless auto_create_index was
+                if collection_name in db.collection_names():
-            # set to False.
+                    cls._collection = db[collection_name]
+                    # The collection already exists, check if its capped
+                    # options match the specified capped options
+                    options = cls._collection.options()
+                    if options.get('max') != max_documents or \
+                            options.get('size') != max_size:
+                        msg = (('Cannot create collection "%s" as a capped '
+                                'collection as it already exists')
+                               % cls._collection)
+                        raise InvalidCollectionError(msg)
+                else:
+                    # Create the collection as a capped collection
+                    opts = {'capped': True, 'size': max_size}
+                    if max_documents:
+                        opts['max'] = max_documents
+                    cls._collection = db.create_collection(
+                        collection_name, **opts
+                    )
+            else:
+                cls._collection = db[collection_name]
             if cls._meta.get('auto_create_index', True):
                 cls.ensure_indexes()
-
         return cls._collection
 
-    @classmethod
-    def _get_capped_collection(cls):
-        """Create a new or get an existing capped PyMongo collection."""
-        db = cls._get_db()
-        collection_name = cls._get_collection_name()
-
-        # Get max document limit and max byte size from meta.
-        max_size = cls._meta.get('max_size') or 10 * 2 ** 20  # 10MB default
-        max_documents = cls._meta.get('max_documents')
-
-        # MongoDB will automatically raise the size to make it a multiple of
-        # 256 bytes. We raise it here ourselves to be able to reliably compare
-        # the options below.
-        if max_size % 256:
-            max_size = (max_size // 256 + 1) * 256
-
-        # If the collection already exists and has different options
-        # (i.e. isn't capped or has different max/size), raise an error.
-        if collection_name in db.collection_names():
-            collection = db[collection_name]
-            options = collection.options()
-            if (
-                options.get('max') != max_documents or
-                options.get('size') != max_size
-            ):
-                raise InvalidCollectionError(
-                    'Cannot create collection "{}" as a capped '
-                    'collection as it already exists'.format(cls._collection)
-                )
-
-            return collection
-
-        # Create a new capped collection.
-        opts = {'capped': True, 'size': max_size}
-        if max_documents:
-            opts['max'] = max_documents
-
-        return db.create_collection(collection_name, **opts)
-
     def to_mongo(self, *args, **kwargs):
         data = super(Document, self).to_mongo(*args, **kwargs)
 

mongoengine/fields.py

@@ -608,9 +608,9 @@ class EmbeddedDocumentField(BaseField):
     """
 
     def __init__(self, document_type, **kwargs):
-        if not (
+        if (
-            isinstance(document_type, six.string_types) or
+            not isinstance(document_type, six.string_types) and
-            issubclass(document_type, EmbeddedDocument)
+            not issubclass(document_type, EmbeddedDocument)
         ):
             self.error('Invalid embedded document class provided to an '
                        'EmbeddedDocumentField')

tests/document/instance.py

@@ -2164,7 +2164,7 @@ class InstanceTest(unittest.TestCase):
         class BlogPost(Document):
             pass
 
-        # Clear old datas
+        # Clear old data
         User.drop_collection()
         BlogPost.drop_collection()
 
@@ -2176,17 +2176,18 @@ class InstanceTest(unittest.TestCase):
         b1 = BlogPost.objects.create()
         b2 = BlogPost.objects.create()
 
-        # in List
+        # Make sure docs are properly identified in a list (__eq__ is used
+        # for the comparison).
         all_user_list = list(User.objects.all())
-
         self.assertTrue(u1 in all_user_list)
         self.assertTrue(u2 in all_user_list)
         self.assertTrue(u3 in all_user_list)
-        self.assertFalse(u4 in all_user_list)  # New object
+        self.assertTrue(u4 not in all_user_list)  # New object
-        self.assertFalse(b1 in all_user_list)  # Other object
+        self.assertTrue(b1 not in all_user_list)  # Other object
-        self.assertFalse(b2 in all_user_list)  # Other object
+        self.assertTrue(b2 not in all_user_list)  # Other object
 
-        # in Dict
+        # Make sure docs can be used as keys in a dict (__hash__ is used
+        # for hashing the docs).
         all_user_dic = {}
         for u in User.objects.all():
             all_user_dic[u] = "OK"
@@ -2198,9 +2199,20 @@ class InstanceTest(unittest.TestCase):
         self.assertEqual(all_user_dic.get(b1, False), False)  # Other object
         self.assertEqual(all_user_dic.get(b2, False), False)  # Other object
 
-        # in Set
+        # Make sure docs are properly identified in a set (__hash__ is used
+        # for hashing the docs).
         all_user_set = set(User.objects.all())
         self.assertTrue(u1 in all_user_set)
+        self.assertTrue(u4 not in all_user_set)
+        self.assertTrue(b1 not in all_user_list)
+        self.assertTrue(b2 not in all_user_list)
+
+        # Make sure duplicate docs aren't accepted in the set
+        self.assertEqual(len(all_user_set), 3)
+        all_user_set.add(u1)
+        all_user_set.add(u2)
+        all_user_set.add(u3)
+        self.assertEqual(len(all_user_set), 3)
 
     def test_picklable(self):
         pickle_doc = PickleTest(number=1, string="One", lists=['1', '2'])