Updated ReferenceField's to optionally store ObjectId strings.

This will become the default in 0.8 (MongoEngine/mongoengine#89)

docs/changelog.rst

@@ -4,6 +4,9 @@ Changelog
 
 Changes in 0.7.X
 =================
+
+- Updated ReferenceField's to optionally store ObjectId strings
+  this will become the default in 0.8 (MongoEngine/mongoengine#89)
 - Added FutureWarning - save will default to `cascade=False` in 0.8
 - Added example of indexing embedded document fields (MongoEngine/mongoengine#75)
 - Fixed ImageField resizing when forcing size (MongoEngine/mongoengine#80)

docs/upgrade.rst

@@ -5,9 +5,13 @@ Upgrading
 0.6 to 0.7
 ==========
 
-Saves will raise a `FutureWarning` if they cascade and cascade hasn't been set to
+Cascade saves
-True.  This is because in 0.8 it will default to False.  If you require cascading
+-------------
-saves then either set it in the `meta` or pass via `save` ::
+
+Saves will raise a `FutureWarning` if they cascade and cascade hasn't been set
+to True.  This is because in 0.8 it will default to False.  If you require
+cascading saves then either set it in the `meta` or pass
+via `save` eg ::
 
     # At the class level:
     class Person(Document):
@@ -16,18 +20,52 @@ saves then either set it in the `meta` or pass via `save` ::
     # Or in code:
     my_document.save(cascade=True)
 
+.. note ::
+    Remember: cascading saves **do not** cascade through lists.
+
+ReferenceFields
+---------------
+
+ReferenceFields now can store references as ObjectId strings instead of DBRefs.
+This will become the default in 0.8 and if `dbref` is not set a `FutureWarning`
+will be raised.
+
+
+To explicitly continue to use DBRefs change the `dbref` flag
+to True ::
+
+   class Person(Document):
+       groups = ListField(ReferenceField(Group, dbref=True))
+
+To migrate to using strings instead of DBRefs you will have to manually
+migrate ::
+
+        # Step 1 - Migrate the model definition
+        class Group(Document):
+            author = ReferenceField(User, dbref=False)
+            members = ListField(ReferenceField(User, dbref=False))
+
+        # Step 2 - Migrate the data
+        for g in Group.objects():
+            g.author = g.author
+            g.members = g.members
+            g.save()
+
+
 0.5 to 0.6
 ==========
 
-Embedded Documents - if you had a `pk` field you will have to rename it from `_id`
+Embedded Documents - if you had a `pk` field you will have to rename it from
-to `pk` as pk is no longer a property of Embedded Documents.
+`_id` to `pk` as pk is no longer a property of Embedded Documents.
 
 Reverse Delete Rules in Embedded Documents, MapFields and DictFields now throw
 an InvalidDocument error as they aren't currently supported.
 
-Document._get_subclasses - Is no longer used and the class method has been removed.
+Document._get_subclasses - Is no longer used and the class method has been
+removed.
 
-Document.objects.with_id - now raises an InvalidQueryError if used with a filter.
+Document.objects.with_id - now raises an InvalidQueryError if used with a
+filter.
 
 FutureWarning - A future warning has been added to all inherited classes that
 don't define `allow_inheritance` in their meta.
@@ -51,11 +89,11 @@ human-readable name for the option.
 PyMongo / MongoDB
 -----------------
 
-map reduce now requires pymongo 1.11+- The pymongo merge_output and reduce_output
+map reduce now requires pymongo 1.11+- The pymongo `merge_output` and
-parameters, have been depreciated.
+`reduce_output` parameters, have been depreciated.
 
-More methods now use map_reduce as db.eval is not supported for sharding as such
+More methods now use map_reduce as db.eval is not supported for sharding as
-the following have been changed:
+such the following have been changed:
 
     * :meth:`~mongoengine.queryset.QuerySet.sum`
     * :meth:`~mongoengine.queryset.QuerySet.average`
@@ -65,8 +103,8 @@ the following have been changed:
 Default collection naming
 -------------------------
 
-Previously it was just lowercase, its now much more pythonic and readable as its
+Previously it was just lowercase, its now much more pythonic and readable as
-lowercase and underscores, previously ::
+its lowercase and underscores, previously ::
 
     class MyAceDocument(Document):
         pass
@@ -102,7 +140,8 @@ Alternatively, you can rename your collections eg ::
 
         failure = False
 
-        collection_names = [d._get_collection_name() for d in _document_registry.values()]
+        collection_names = [d._get_collection_name()
+                            for d in _document_registry.values()]
 
         for new_style_name in collection_names:
             if not new_style_name:  # embedded documents don't have collections
@@ -120,7 +159,8 @@ Alternatively, you can rename your collections eg ::
                         old_style_name, new_style_name)
                 else:
                     db[old_style_name].rename(new_style_name)
-                    print "Renamed:  %s to %s" % (old_style_name, new_style_name)
+                    print "Renamed:  %s to %s" % (old_style_name,
+                                                  new_style_name)
 
         if failure:
             print "Upgrading  collection names failed"

mongoengine/base.py

@@ -470,6 +470,8 @@ class ObjectIdField(BaseField):
     """
 
     def to_python(self, value):
+        if not isinstance(value, ObjectId):
+            value = ObjectId(value)
         return value
 
     def to_mongo(self, value):

mongoengine/dereference.py

@@ -39,9 +39,26 @@ class DeReference(object):
                 doc_type = doc_type.field
 
             if isinstance(doc_type, ReferenceField):
+                field = doc_type
                 doc_type = doc_type.document_type
-                if all([i.__class__ == doc_type for i in items]):
+                is_list = not hasattr(items, 'items')
+
+                if is_list and all([i.__class__ == doc_type for i in items]):
                     return items
+                elif not is_list and all([i.__class__ == doc_type
+                                         for i in items.values()]):
+                    return items
+                elif not field.dbref:
+                    if not hasattr(items, 'items'):
+                        items = [field.to_python(v)
+                             if not isinstance(v, (DBRef, Document)) else v
+                             for v in items]
+                    else:
+                        items = dict([
+                            (k, field.to_python(v))
+                            if not isinstance(v, (DBRef, Document)) else (k, v)
+                            for k, v in items.iteritems()]
+                        )
 
         self.reference_map = self._find_references(items)
         self.object_map = self._fetch_objects(doc_type=doc_type)

mongoengine/document.py

@@ -277,6 +277,9 @@ class Document(BaseDocument):
             if not ref or isinstance(ref, DBRef):
                 continue
 
+            if not getattr(ref, '_changed_fields', True):
+                continue
+
             ref_id = "%s,%s" % (ref.__class__.__name__, str(ref._data))
             if ref and ref_id not in _refs:
                 if warn_cascade:

mongoengine/fields.py

@@ -693,7 +693,8 @@ class ReferenceField(BaseField):
       * NULLIFY     - Updates the reference to null.
       * CASCADE     - Deletes the documents associated with the reference.
       * DENY        - Prevent the deletion of the reference object.
-      * PULL        - Pull the reference from a :class:`~mongoengine.ListField` of references
+      * PULL        - Pull the reference from a :class:`~mongoengine.ListField`
+                      of references
 
     Alternative syntax for registering delete rules (useful when implementing
     bi-directional delete rules)
@@ -709,9 +710,12 @@ class ReferenceField(BaseField):
     .. versionchanged:: 0.5 added `reverse_delete_rule`
     """
 
-    def __init__(self, document_type, reverse_delete_rule=DO_NOTHING, **kwargs):
+    def __init__(self, document_type, dbref=None,
+                 reverse_delete_rule=DO_NOTHING, **kwargs):
         """Initialises the Reference Field.
 
+        :param dbref:  Store the reference as :class:`~pymongo.dbref.DBRef`
+          or as the :class:`~pymongo.objectid.ObjectId`.id .
         :param reverse_delete_rule: Determines what to do when the referring
           object is deleted
         """
@@ -719,6 +723,13 @@ class ReferenceField(BaseField):
             if not issubclass(document_type, (Document, basestring)):
                 self.error('Argument to ReferenceField constructor must be a '
                            'document class or a string')
+
+        if dbref is None:
+            msg = ("ReferenceFields will default to using ObjectId "
+                   " strings in 0.8, set DBRef=True if this isn't desired")
+            warnings.warn(msg, FutureWarning)
+
+        self.dbref = dbref if dbref is not None else True  # To change in 0.8
         self.document_type_obj = document_type
         self.reverse_delete_rule = reverse_delete_rule
         super(ReferenceField, self).__init__(**kwargs)
@@ -741,8 +752,9 @@ class ReferenceField(BaseField):
 
         # Get value from document instance if available
         value = instance._data.get(self.name)
+
         # Dereference DBRefs
-        if isinstance(value, (DBRef)):
+        if isinstance(value, DBRef):
             value = self.document_type._get_db().dereference(value)
             if value is not None:
                 instance._data[self.name] = self.document_type._from_son(value)
@@ -751,6 +763,10 @@ class ReferenceField(BaseField):
 
     def to_mongo(self, document):
         if isinstance(document, DBRef):
+            if not self.dbref:
+                return "%s" % DBRef.id
+            return document
+        elif not self.dbref and isinstance(document, basestring):
             return document
 
         id_field_name = self.document_type._meta['id_field']
@@ -766,17 +782,30 @@ class ReferenceField(BaseField):
             id_ = document
 
         id_ = id_field.to_mongo(id_)
+        if self.dbref:
             collection = self.document_type._get_collection_name()
             return DBRef(collection, id_)
 
+        return "%s" % id_
+
+    def to_python(self, value):
+        """Convert a MongoDB-compatible type to a Python type.
+        """
+        if (not self.dbref and
+            not isinstance(value, (DBRef, Document, EmbeddedDocument))):
+            collection = self.document_type._get_collection_name()
+            value = DBRef(collection, self.document_type.id.to_python(value))
+        return value
+
     def prepare_query_value(self, op, value):
         if value is None:
             return None
         return self.to_mongo(value)
 
     def validate(self, value):
+
         if not isinstance(value, (self.document_type, DBRef)):
-            self.error('A ReferenceField only accepts DBRef')
+            self.error("A ReferenceField only accepts DBRef or documents")
 
         if isinstance(value, Document) and value.id is None:
             self.error('You can only reference documents once they have been '

tests/test_all_warnings.py

@@ -37,6 +37,28 @@ class TestWarnings(unittest.TestCase):
         self.assertEqual(FutureWarning, warning["category"])
         self.assertTrue("InheritedClass" in str(warning["message"]))
 
+    def test_dbref_reference_field_future_warning(self):
+
+        class Person(Document):
+            name = StringField()
+            parent = ReferenceField('self')
+
+        Person.drop_collection()
+
+        p1 = Person()
+        p1.parent = None
+        p1.save()
+
+        p2 = Person(name="Wilson Jr")
+        p2.parent = p1
+        p2.save(cascade=False)
+
+        self.assertEqual(len(self.warning_list), 1)
+        warning = self.warning_list[0]
+        self.assertEqual(FutureWarning, warning["category"])
+        self.assertTrue("ReferenceFields will default to using ObjectId"
+                        in str(warning["message"]))
+
     def test_document_save_cascade_future_warning(self):
 
         class Person(Document):

tests/test_dereference.py

@@ -64,6 +64,130 @@ class FieldTest(unittest.TestCase):
         User.drop_collection()
         Group.drop_collection()
 
+    def test_list_item_dereference_dref_false(self):
+        """Ensure that DBRef items in ListFields are dereferenced.
+        """
+        class User(Document):
+            name = StringField()
+
+        class Group(Document):
+            members = ListField(ReferenceField(User, dbref=False))
+
+        User.drop_collection()
+        Group.drop_collection()
+
+        for i in xrange(1, 51):
+            user = User(name='user %s' % i)
+            user.save()
+
+        group = Group(members=User.objects)
+        group.save()
+
+        with query_counter() as q:
+            self.assertEqual(q, 0)
+
+            group_obj = Group.objects.first()
+            self.assertEqual(q, 1)
+
+            [m for m in group_obj.members]
+            self.assertEqual(q, 2)
+
+        # Document select_related
+        with query_counter() as q:
+            self.assertEqual(q, 0)
+
+            group_obj = Group.objects.first().select_related()
+
+            self.assertEqual(q, 2)
+            [m for m in group_obj.members]
+            self.assertEqual(q, 2)
+
+        # Queryset select_related
+        with query_counter() as q:
+            self.assertEqual(q, 0)
+            group_objs = Group.objects.select_related()
+            self.assertEqual(q, 2)
+            for group_obj in group_objs:
+                [m for m in group_obj.members]
+                self.assertEqual(q, 2)
+
+        User.drop_collection()
+        Group.drop_collection()
+
+    def test_handle_old_style_references(self):
+        """Ensure that DBRef items in ListFields are dereferenced.
+        """
+        class User(Document):
+            name = StringField()
+
+        class Group(Document):
+            members = ListField(ReferenceField(User, dbref=True))
+
+        User.drop_collection()
+        Group.drop_collection()
+
+        for i in xrange(1, 26):
+            user = User(name='user %s' % i)
+            user.save()
+
+        group = Group(members=User.objects)
+        group.save()
+
+        group = Group._get_collection().find_one()
+
+        # Update the model to change the reference
+        class Group(Document):
+            members = ListField(ReferenceField(User, dbref=False))
+
+        group = Group.objects.first()
+        group.members.append(User(name="String!").save())
+        group.save()
+
+        group = Group.objects.first()
+        self.assertEqual(group.members[0].name, 'user 1')
+        self.assertEqual(group.members[-1].name, 'String!')
+
+    def test_migrate_references(self):
+        """Example of migrating ReferenceField storage
+        """
+
+        # Create some sample data
+        class User(Document):
+            name = StringField()
+
+        class Group(Document):
+            author = ReferenceField(User, dbref=True)
+            members = ListField(ReferenceField(User, dbref=True))
+
+        User.drop_collection()
+        Group.drop_collection()
+
+        user = User(name="Ross").save()
+        group = Group(author=user, members=[user]).save()
+
+        raw_data = Group._get_collection().find_one()
+        self.assertTrue(isinstance(raw_data['author'], DBRef))
+        self.assertTrue(isinstance(raw_data['members'][0], DBRef))
+
+        # Migrate the model definition
+        class Group(Document):
+            author = ReferenceField(User, dbref=False)
+            members = ListField(ReferenceField(User, dbref=False))
+
+        # Migrate the data
+        for g in Group.objects():
+            g.author = g.author
+            g.members = g.members
+            g.save()
+
+        group = Group.objects.first()
+        self.assertEqual(group.author, user)
+        self.assertEqual(group.members, [user])
+
+        raw_data = Group._get_collection().find_one()
+        self.assertTrue(isinstance(raw_data['author'], basestring))
+        self.assertTrue(isinstance(raw_data['members'][0], basestring))
+
     def test_recursive_reference(self):
         """Ensure that ReferenceFields can reference their own documents.
         """

tests/test_fields.py

@@ -7,7 +7,7 @@ import tempfile
 
 from decimal import Decimal
 
-from bson import Binary
+from bson import Binary, DBRef
 import gridfs
 
 from nose.plugins.skip import SkipTest
@@ -1086,6 +1086,42 @@ class FieldTest(unittest.TestCase):
         User.drop_collection()
         BlogPost.drop_collection()
 
+    def test_dbref_reference_fields(self):
+
+        class Person(Document):
+            name = StringField()
+            parent = ReferenceField('self', dbref=True)
+
+        Person.drop_collection()
+
+        p1 = Person(name="John").save()
+        Person(name="Ross", parent=p1).save()
+
+        col = Person._get_collection()
+        data = col.find_one({'name': 'Ross'})
+        self.assertEqual(data['parent'], DBRef('person', p1.pk))
+
+        p = Person.objects.get(name="Ross")
+        self.assertEqual(p.parent, p1)
+
+    def test_str_reference_fields(self):
+
+        class Person(Document):
+            name = StringField()
+            parent = ReferenceField('self', dbref=False)
+
+        Person.drop_collection()
+
+        p1 = Person(name="John").save()
+        Person(name="Ross", parent=p1).save()
+
+        col = Person._get_collection()
+        data = col.find_one({'name': 'Ross'})
+        self.assertEqual(data['parent'], "%s" % p1.pk)
+
+        p = Person.objects.get(name="Ross")
+        self.assertEqual(p.parent, p1)
+
     def test_list_item_dereference(self):
         """Ensure that DBRef items in ListFields are dereferenced.
         """
@@ -1122,6 +1158,7 @@ class FieldTest(unittest.TestCase):
             boss = ReferenceField('self')
             friends = ListField(ReferenceField('self'))
 
+        Employee.drop_collection()
         bill = Employee(name='Bill Lumbergh')
         bill.save()
 
@@ -1245,7 +1282,41 @@ class FieldTest(unittest.TestCase):
 
         class BlogPost(Document):
             title = StringField()
-            author = ReferenceField(Member)
+            author = ReferenceField(Member, dbref=False)
+
+        Member.drop_collection()
+        BlogPost.drop_collection()
+
+        m1 = Member(user_num=1)
+        m1.save()
+        m2 = Member(user_num=2)
+        m2.save()
+
+        post1 = BlogPost(title='post 1', author=m1)
+        post1.save()
+
+        post2 = BlogPost(title='post 2', author=m2)
+        post2.save()
+
+        post = BlogPost.objects(author=m1).first()
+        self.assertEqual(post.id, post1.id)
+
+        post = BlogPost.objects(author=m2).first()
+        self.assertEqual(post.id, post2.id)
+
+        Member.drop_collection()
+        BlogPost.drop_collection()
+
+    def test_reference_query_conversion_dbref(self):
+        """Ensure that ReferenceFields can be queried using objects and values
+        of the type of the primary key of the referenced object.
+        """
+        class Member(Document):
+            user_num = IntField(primary_key=True)
+
+        class BlogPost(Document):
+            title = StringField()
+            author = ReferenceField(Member, dbref=True)
 
         Member.drop_collection()
         BlogPost.drop_collection()