fix for nested MapFields

[damoxc]

When using nested MapFields from a document loaded from the database, the
nested dictionaries aren't converted to BaseDict, so changes aren't
marked.

This also changes the way BaseDict's mark things as changed, before they'd mark
the entire dictionary as changed, which may well be undesirable if you have large
dictionaries but are only changing a single field nested at some depth.

This also includes a change when marking a field as changed to ensure that
nested fields aren't included in a $set query if a parent is already marked
as changed. Not sure if this could occur but it prevents breakage if it does.

Mostly looking for comments on this change, happy to fix up / add tests if this looks
good.

[rozza]

Couldn't we convert to nested dicts to BaseDict when we fetch them?

[damoxc]

Instead of doing it lazily when first accessed? What benefit does that have versus doing it lazily?

[rozza]

Sorry I meant when accessing them. Ok firstly this needs tests :)

I'll also add comments to the pr.

[hai-ld]

Hi @damoxc

Where do you specifically implement the following

This also includes a change when marking a field as changed to ensure that
nested fields aren't included in a $set query if a parent is already marked
as changed. Not sure if this could occur but it prevents breakage if it does.

Because it seems that the code doesn't work as you intended. See the following snippet for detailed explanation. I tested it with pymongo==2.8.1 and mongoengine==0.9.0.

from mongoengine import connect
from mongoengine import Document, DictField, StringField, DynamicDocument

class TestDoc(DynamicDocument):
    uid = StringField(primary_key=True)
    num_map = DictField()

db = connect('test')
TestDoc.objects.delete()

# Insert doc without num_map as an empty dict
db.test.test_doc.insert({"_id": "123"})

# Load inserted doc
doc = TestDoc.objects[0]
# here _changed_fields is already ['num_map'] because of changes made in pull request 723
# https://github.com/MongoEngine/mongoengine/pull/723
# Specifically, commit 1e6a3163 https://github.com/MongoEngine/mongoengine/commit/1e6a3163
print doc._changed_fields

# Change inserted doc
doc["num_map"]["one"] = 1

# here we expect _changed_fields to be just ['num_map']
# however, it's ['num_map', 'num_map.one'] instead...
print doc._changed_fields

# ...which causes this line to fail with this exception
# local/lib/python2.7/site-packages/mongoengine/document.pyc in save(self, force_insert, validate, clean, write_concern, cascade, cascade_kwargs, _refs, save_condition, **kwargs)
#     365                 message = u'Tried to save duplicate unique keys (%s)'
#     366                 raise NotUniqueError(message % unicode(err))
# --> 367             raise OperationError(message % unicode(err))
#     368         id_field = self._meta['id_field']
#     369         if created or id_field not in self._meta.get('shard_key', []):

# OperationError: Could not save document (Cannot update 'num_map' and 'num_map.one' at the same time)
doc.save()

[damoxc]

It certainly did at the time of writing 😃

Took a little bit of remembering but I think this is occurring in the module mongoengine/base/document.py and the function _nestable_types_changed_fields

[hai-ld]

@damoxc @rozza As commented in issue #998, it was fixed in #927 by changing _mark_as_changed in base/document.py. I wonder when the next official release (that contains the fix) is.