_dynamic field updates - fixed pickling and creation order

Dynamic fields are ordered based on creation and stored in _fields_ordered (#396)
Fixed pickling dynamic documents `_dynamic_fields` (#387)

docs/changelog.rst

@@ -4,6 +4,8 @@ Changelog
 
 Changes in 0.8.3
 ================
+- Dynamic fields are ordered based on creation and stored in _fields_ordered (#396)
+- Fixed pickling dynamic documents `_dynamic_fields` (#387)
 - Fixed ListField setslice and delslice dirty tracking (#390)
 - Added Django 1.5 PY3 support (#392)
 - Added match ($elemMatch) support for EmbeddedDocuments (#379)

docs/guide/defining-documents.rst

@@ -54,7 +54,7 @@ be saved ::
 
    There is one caveat on Dynamic Documents: fields cannot start with `_`
 
-Dynamic fields are stored in alphabetical order *after* any declared fields.
+Dynamic fields are stored in creation order *after* any declared fields.
 
 Fields
 ======

docs/upgrade.rst

@@ -2,6 +2,16 @@
 Upgrading
 #########
 
+
+0.8.2 to 0.8.2
+**************
+
+Minor change that may impact users:
+
+DynamicDocument fields are now stored in creation order after any declared
+fields.  Previously they were stored alphabetically.
+
+
 0.7 to 0.8
 **********
 

mongoengine/base/document.py

@@ -42,6 +42,9 @@ class BaseDocument(object):
             # Combine positional arguments with named arguments.
             # We only want named arguments.
             field = iter(self._fields_ordered)
+            # If its an automatic id field then skip to the first defined field
+            if self._auto_id_field:
+                next(field)
             for value in args:
                 name = next(field)
                 if name in values:
@@ -51,6 +54,7 @@ class BaseDocument(object):
         signals.pre_init.send(self.__class__, document=self, values=values)
 
         self._data = {}
+        self._dynamic_fields = SON()
 
         # Assign default values to instance
         for key, field in self._fields.iteritems():
@@ -61,7 +65,6 @@ class BaseDocument(object):
 
         # Set passed values after initialisation
         if self._dynamic:
-            self._dynamic_fields = {}
             dynamic_data = {}
             for key, value in values.iteritems():
                 if key in self._fields or key == '_id':
@@ -116,6 +119,7 @@ class BaseDocument(object):
                 field = DynamicField(db_field=name)
                 field.name = name
                 self._dynamic_fields[name] = field
+                self._fields_ordered += (name,)
 
             if not name.startswith('_'):
                 value = self.__expand_dynamic_values(name, value)
@@ -142,7 +146,8 @@ class BaseDocument(object):
 
     def __getstate__(self):
         data = {}
-        for k in ('_changed_fields', '_initialised', '_created'):
+        for k in ('_changed_fields', '_initialised', '_created',
+                  '_dynamic_fields', '_fields_ordered'):
             if hasattr(self, k):
                 data[k] = getattr(self, k)
         data['_data'] = self.to_mongo()
@@ -151,21 +156,21 @@ class BaseDocument(object):
     def __setstate__(self, data):
         if isinstance(data["_data"], SON):
             data["_data"] = self.__class__._from_son(data["_data"])._data
-        for k in ('_changed_fields', '_initialised', '_created', '_data'):
+        for k in ('_changed_fields', '_initialised', '_created', '_data',
+                  '_fields_ordered', '_dynamic_fields'):
             if k in data:
                 setattr(self, k, data[k])
+        for k in data.get('_dynamic_fields').keys():
+            setattr(self, k, data["_data"].get(k))
 
     def __iter__(self):
-        if 'id' in self._fields and 'id' not in self._fields_ordered:
-            return iter(('id', ) + self._fields_ordered)
-
         return iter(self._fields_ordered)
 
     def __getitem__(self, name):
         """Dictionary-style field access, return a field's value if present.
         """
         try:
-            if name in self._fields:
+            if name in self._fields_ordered:
                 return getattr(self, name)
         except AttributeError:
             pass
@@ -241,6 +246,8 @@ class BaseDocument(object):
         for field_name in self:
             value = self._data.get(field_name, None)
             field = self._fields.get(field_name)
+            if field is None and self._dynamic:
+                field = self._dynamic_fields.get(field_name)
 
             if value is not None:
                 value = field.to_mongo(value)
@@ -265,15 +272,6 @@ class BaseDocument(object):
            not self._meta.get('allow_inheritance', ALLOW_INHERITANCE)):
             data.pop('_cls')
 
-        if not self._dynamic:
-            return data
-
-        # Sort dynamic fields by key
-        dynamic_fields = sorted(self._dynamic_fields.iteritems(),
-                                key=operator.itemgetter(0))
-        for name, field in dynamic_fields:
-            data[name] = field.to_mongo(self._data.get(name, None))
-
         return data
 
     def validate(self, clean=True):
@@ -289,11 +287,8 @@ class BaseDocument(object):
                 errors[NON_FIELD_ERRORS] = error
 
         # Get a list of tuples of field names and their current values
-        fields = [(field, self._data.get(name))
+        fields = [(self._fields.get(name, self._dynamic_fields.get(name)),
-                  for name, field in self._fields.items()]
+                   self._data.get(name)) for name in self._fields_ordered]
-        if self._dynamic:
-            fields += [(field, self._data.get(name))
-                       for name, field in self._dynamic_fields.items()]
 
         EmbeddedDocumentField = _import_class("EmbeddedDocumentField")
         GenericEmbeddedDocumentField = _import_class("GenericEmbeddedDocumentField")
@@ -406,11 +401,7 @@ class BaseDocument(object):
                 return _changed_fields
             inspected.add(self.id)
 
-        field_list = self._fields.copy()
+        for field_name in self._fields_ordered:
-        if self._dynamic:
-            field_list.update(self._dynamic_fields)
-
-        for field_name in field_list:
 
             db_field_name = self._db_field_map.get(field_name, field_name)
             key = '%s.' % db_field_name
@@ -450,7 +441,6 @@ class BaseDocument(object):
         doc = self.to_mongo()
 
         set_fields = self._get_changed_fields()
-        set_data = {}
         unset_data = {}
         parts = []
         if hasattr(self, '_changed_fields'):

mongoengine/base/metaclasses.py

@@ -91,11 +91,12 @@ class DocumentMetaclass(type):
         attrs['_fields'] = doc_fields
         attrs['_db_field_map'] = dict([(k, getattr(v, 'db_field', k))
                                       for k, v in doc_fields.iteritems()])
+        attrs['_reverse_db_field_map'] = dict(
+            (v, k) for k, v in attrs['_db_field_map'].iteritems())
+
         attrs['_fields_ordered'] = tuple(i[1] for i in sorted(
                                          (v.creation_counter, v.name)
                                          for v in doc_fields.itervalues()))
-        attrs['_reverse_db_field_map'] = dict(
-            (v, k) for k, v in attrs['_db_field_map'].iteritems())
 
         #
         # Set document hierarchy
@@ -358,12 +359,18 @@ class TopLevelDocumentMetaclass(DocumentMetaclass):
                     new_class.id = field
 
         # Set primary key if not defined by the document
+        new_class._auto_id_field = False
         if not new_class._meta.get('id_field'):
+            new_class._auto_id_field = True
             new_class._meta['id_field'] = 'id'
             new_class._fields['id'] = ObjectIdField(db_field='_id')
             new_class._fields['id'].name = 'id'
             new_class.id = new_class._fields['id']
 
+        # Prepend id field to _fields_ordered
+        if 'id' in new_class._fields and 'id' not in new_class._fields_ordered:
+            new_class._fields_ordered = ('id', ) + new_class._fields_ordered
+
         # Merge in exceptions with parent hierarchy
         exceptions_to_merge = (DoesNotExist, MultipleObjectsReturned)
         module = attrs.get('__module__')

mongoengine/document.py

@@ -460,11 +460,8 @@ class Document(BaseDocument):
         else:
             msg = "Reloaded document has been deleted"
             raise OperationError(msg)
-        for field in self._fields:
+        for field in self._fields_ordered:
             setattr(self, field, self._reload(field, obj[field]))
-        if self._dynamic:
-            for name in self._dynamic_fields.keys():
-                setattr(self, name, self._reload(name, obj._data[name]))
         self._changed_fields = obj._changed_fields
         self._created = False
         return obj

tests/document/delta.py

@@ -3,6 +3,7 @@ import sys
 sys.path[0:0] = [""]
 import unittest
 
+from bson import SON
 from mongoengine import *
 from mongoengine.connection import get_db
 
@@ -613,13 +614,13 @@ class DeltaTest(unittest.TestCase):
         Person.drop_collection()
 
         p = Person(name="James", age=34)
-        self.assertEqual(p._delta(), ({'age': 34, 'name': 'James',
+        self.assertEqual(p._delta(), (
-                                       '_cls': 'Person'}, {}))
+            SON([('_cls', 'Person'), ('name', 'James'), ('age', 34)]), {}))
 
         p.doc = 123
         del(p.doc)
-        self.assertEqual(p._delta(), ({'age': 34, 'name': 'James',
+        self.assertEqual(p._delta(), (
-                                       '_cls': 'Person'}, {'doc': 1}))
+            SON([('_cls', 'Person'), ('name', 'James'), ('age', 34)]), {}))
 
         p = Person()
         p.name = "Dean"
@@ -631,14 +632,14 @@ class DeltaTest(unittest.TestCase):
         self.assertEqual(p._get_changed_fields(), ['age'])
         self.assertEqual(p._delta(), ({'age': 24}, {}))
 
-        p = self.Person.objects(age=22).get()
+        p = Person.objects(age=22).get()
         p.age = 24
         self.assertEqual(p.age, 24)
         self.assertEqual(p._get_changed_fields(), ['age'])
         self.assertEqual(p._delta(), ({'age': 24}, {}))
 
         p.save()
-        self.assertEqual(1, self.Person.objects(age=24).count())
+        self.assertEqual(1, Person.objects(age=24).count())
 
     def test_dynamic_delta(self):
 

tests/document/instance.py

@@ -10,7 +10,8 @@ import uuid
 
 from datetime import datetime
 from bson import DBRef
-from tests.fixtures import PickleEmbedded, PickleTest, PickleSignalsTest
+from tests.fixtures import (PickleEmbedded, PickleTest, PickleSignalsTest,
+                            PickleDyanmicEmbedded, PickleDynamicTest)
 
 from mongoengine import *
 from mongoengine.errors import (NotRegistered, InvalidDocumentError,
@@ -1827,6 +1828,29 @@ class InstanceTest(unittest.TestCase):
         self.assertEqual(pickle_doc.string, "Two")
         self.assertEqual(pickle_doc.lists, ["1", "2", "3"])
 
+    def test_dynamic_document_pickle(self):
+
+        pickle_doc = PickleDynamicTest(name="test", number=1, string="One", lists=['1', '2'])
+        pickle_doc.embedded = PickleDyanmicEmbedded(foo="Bar")
+        pickled_doc = pickle.dumps(pickle_doc)  # make sure pickling works even before the doc is saved
+
+        pickle_doc.save()
+
+        pickled_doc = pickle.dumps(pickle_doc)
+        resurrected = pickle.loads(pickled_doc)
+
+        self.assertEqual(resurrected, pickle_doc)
+        self.assertEqual(resurrected._fields_ordered,
+                         pickle_doc._fields_ordered)
+        self.assertEqual(resurrected._dynamic_fields.keys(),
+                         pickle_doc._dynamic_fields.keys())
+
+        self.assertEqual(resurrected.embedded, pickle_doc.embedded)
+        self.assertEqual(resurrected.embedded._fields_ordered,
+                         pickle_doc.embedded._fields_ordered)
+        self.assertEqual(resurrected.embedded._dynamic_fields.keys(),
+                         pickle_doc.embedded._dynamic_fields.keys())
+
     def test_picklable_on_signals(self):
         pickle_doc = PickleSignalsTest(number=1, string="One", lists=['1', '2'])
         pickle_doc.embedded = PickleEmbedded()
@@ -2289,6 +2313,16 @@ class InstanceTest(unittest.TestCase):
         self.assertEqual(person.name, "Test User")
         self.assertEqual(person.age, 42)
 
+    def test_mixed_creation_dynamic(self):
+        """Ensure that document may be created using mixed arguments.
+        """
+        class Person(DynamicDocument):
+            name = StringField()
+
+        person = Person("Test User", age=42)
+        self.assertEqual(person.name, "Test User")
+        self.assertEqual(person.age, 42)
+
     def test_bad_mixed_creation(self):
         """Ensure that document gives correct error when duplicating arguments
         """

tests/fixtures.py

@@ -17,6 +17,14 @@ class PickleTest(Document):
     photo = FileField()
 
 
+class PickleDyanmicEmbedded(DynamicEmbeddedDocument):
+    date = DateTimeField(default=datetime.now)
+
+
+class PickleDynamicTest(DynamicDocument):
+    number = IntField()
+
+
 class PickleSignalsTest(Document):
     number = IntField()
     string = StringField(choices=(('One', '1'), ('Two', '2')))