From af86aee9700504458ae945914a2b9412c5da8ea6 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Wed, 10 Jul 2013 10:57:24 +0000 Subject: [PATCH] _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 | 2 ++ docs/guide/defining-documents.rst | 2 +- docs/upgrade.rst | 10 +++++++ mongoengine/base/document.py | 44 ++++++++++++------------------- mongoengine/base/metaclasses.py | 11 ++++++-- mongoengine/document.py | 5 +--- tests/document/delta.py | 13 ++++----- tests/document/instance.py | 36 ++++++++++++++++++++++++- tests/fixtures.py | 8 ++++++ 9 files changed, 90 insertions(+), 41 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 27d51a43..78deafb9 100644 --- a/docs/changelog.rst +++ b/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) diff --git a/docs/guide/defining-documents.rst b/docs/guide/defining-documents.rst index a61d8fe8..a50450e3 100644 --- a/docs/guide/defining-documents.rst +++ b/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 ====== diff --git a/docs/upgrade.rst b/docs/upgrade.rst index c3d31824..b8864b0d 100644 --- a/docs/upgrade.rst +++ b/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 ********** diff --git a/mongoengine/base/document.py b/mongoengine/base/document.py index ca154a29..04b0c050 100644 --- a/mongoengine/base/document.py +++ b/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)) - for name, field in self._fields.items()] - if self._dynamic: - fields += [(field, self._data.get(name)) - for name, field in self._dynamic_fields.items()] + fields = [(self._fields.get(name, self._dynamic_fields.get(name)), + self._data.get(name)) for name in self._fields_ordered] 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() - if self._dynamic: - field_list.update(self._dynamic_fields) - - for field_name in field_list: + for field_name in self._fields_ordered: 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'): diff --git a/mongoengine/base/metaclasses.py b/mongoengine/base/metaclasses.py index 444d9a25..ff5afddf 100644 --- a/mongoengine/base/metaclasses.py +++ b/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__') diff --git a/mongoengine/document.py b/mongoengine/document.py index ab8fa2ac..d0c9e616 100644 --- a/mongoengine/document.py +++ b/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 diff --git a/tests/document/delta.py b/tests/document/delta.py index 16ab609b..3656d9e3 100644 --- a/tests/document/delta.py +++ b/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', - '_cls': 'Person'}, {})) + self.assertEqual(p._delta(), ( + SON([('_cls', 'Person'), ('name', 'James'), ('age', 34)]), {})) p.doc = 123 del(p.doc) - self.assertEqual(p._delta(), ({'age': 34, 'name': 'James', - '_cls': 'Person'}, {'doc': 1})) + self.assertEqual(p._delta(), ( + 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): diff --git a/tests/document/instance.py b/tests/document/instance.py index 81734aa0..e85c9d86 100644 --- a/tests/document/instance.py +++ b/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 """ diff --git a/tests/fixtures.py b/tests/fixtures.py index e2070443..f1344d7c 100644 --- a/tests/fixtures.py +++ b/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')))